Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ClientID cannot be specified by user with Hydra v2.0.1 (incompatible with v2) #117

Open
3 of 6 tasks
nbrns opened this issue Nov 11, 2022 · 22 comments
Open
3 of 6 tasks
Labels
bug Something is not working.

Comments

@nbrns
Copy link

nbrns commented Nov 11, 2022

Preflight checklist

Describe the bug

I deploy Hydra v2.0.1 to k8s via Helm and I really love the idea to create OAuth2 clients via a CR.
However, with Hydra v2 it is no longer possible to change

ERROR	controllers.OAuth2Client	error processing client authentication-broker-oauth2-client/taal-evolution 	{"oauth2client": "register", "error": "POST http://authentication-broker-hydra-admin:4445/admin/clients http request returned unexpected status code: 400 Bad Request"}

on Hydra side:

level=info msg=An error occurred while handling a request audience=application error=map[debug: message:The request was malformed or contained invalid parameters reason:It is no longer possible to set an OAuth2 Client ID as a user. The system will generate a unique ID for you. status:Bad Request status_code:400] http_request=map[headers:map[accept:application/json accept-encoding:gzip content-length:407 content-type:application/json user-agent:Go-http-client/1.1] host:authentication-broker-hydra-admin:4445 method:POST path:/admin/clients query:<nil> remote:10.244.5.33:44274 scheme:http] http_response=map[status_code:400] service_name=Ory Hydra service_version=v2.0.1

Reproducing the bug

Deploy Hydra > v.2.0.0 and create a OAuth2Client CR for reconciliation by Hydra-Maester

Relevant log output

No response

Relevant configuration

No response

Version

v0.0.26

On which operating system are you observing this issue?

No response

In which environment are you deploying?

No response

Additional Context

No response

@nbrns nbrns added the bug Something is not working. label Nov 11, 2022
@Demonsthere
Copy link
Collaborator

Hello there,
Thanks for the report. Did you verify if the proper endpoint is being used by the controller? We had a similar case reported here ory/k8s#543

@ghost
Copy link

ghost commented Nov 24, 2022

Hello there, Thanks for the report. Did you verify if the proper endpoint is being used by the controller? We had a similar case reported here ory/k8s#543

I did very to hit correct end point. I started suddenly getting this error probably after getting a hydra chart update.
If you run a sample below which includes client_id the request will fail with the reported error. Remove client_id & it works.

My logs clearly show same error

Sample of a failing request is below

curl -X POST \
  'http://localhost:4203/admin/clients' \
  --header 'Accept: */*' \
  --header 'Content-Type: application/json' \
  --data-raw '{
  "client_id": "asdasdasdasd",
  "audience": [
    "toggleblock-user",
    "toggleblock-admin"
  ],
  "backchannel_logout_session_required": true,
  "client_secret_expires_at": 0,
  "frontchannel_logout_session_required": true,
  "grant_types": [
    "client_credentials",
    "implicit",
    "authorization_code",
    "refresh_token"
  ],
  "post_logout_redirect_uris": [
    "https://www.toggleblock.local:4200"
  ],
  "redirect_uris": [
    "https://www.toggleblock.local:4200"
  ],
  "response_types": [
    "id_token",
    "code",
    "token"
  ],
  "scope": "openid offline_access",
  "token_endpoint_auth_method": "none"
}'

@yasha-spare
Copy link

yasha-spare commented Dec 7, 2022

I'm seeing this error too. My Hydra Maester logs look like:

2022-12-07T18:57:34.134Z	INFO	controllers.OAuth2Client	using default client
2022-12-07T18:57:34.138Z	INFO	controllers.OAuth2Client	using default client
2022-12-07T18:57:34.146Z	INFO	controllers.OAuth2Client	using default client
2022-12-07T18:57:34.148Z	ERROR	controllers.OAuth2Client	error processing client oauth2-client-matching-service/main 	{"oauth2client": "register", "error": "POST http://hydra-admin:4445/admin/clients http request returned unexpected status code: 400 Bad Request"}
github.com/go-logr/zapr.(*zapLogger).Error
	/go/pkg/mod/github.com/go-logr/[email protected]/zapr.go:132
github.com/ory/hydra-maester/controllers.(*OAuth2ClientReconciler).updateReconciliationStatusError
	/go/src/github.com/ory/hydra-maester/controllers/oauth2client_controller.go:367
github.com/ory/hydra-maester/controllers.(*OAuth2ClientReconciler).registerOAuth2Client
	/go/src/github.com/ory/hydra-maester/controllers/oauth2client_controller.go:271
github.com/ory/hydra-maester/controllers.(*OAuth2ClientReconciler).Reconcile
	/go/src/github.com/ory/hydra-maester/controllers/oauth2client_controller.go:238
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:298
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:253
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func1.2
	/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:216
k8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext.func1
	/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:185
k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1
	/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:155
k8s.io/apimachinery/pkg/util/wait.BackoffUntil
	/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:156
k8s.io/apimachinery/pkg/util/wait.JitterUntil
	/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:133
k8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext
	/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:185
k8s.io/apimachinery/pkg/util/wait.UntilWithContext
	/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:99

And the corresponding Hydra logs look like:

time=2022-12-07T18:50:02Z level=info msg=An error occurred while handling a request audience=application error=map[debug: message:The request was malformed or contained invalid parameters reason:It is no longer possible to set an OAuth2 Client ID as a user. The system will generate a unique ID for you. status:Bad Request status_code:400] http_request=map[headers:map[accept:application/json accept-encoding:gzip content-length:256 content-type:application/json user-agent:Go-http-client/1.1] host:hydra-admin:4445 method:POST path:/admin/clients query:<nil> remote:<ip_redacted>:44702 scheme:http] http_response=map[status_code:400] service_name=Ory Hydra service_version=v2.0.1

Seems like Hydra Maester is trying to POST path:/admin/clients while providing a client id, which is not valid in Hydra 2.

I have Hydra Maester and Hydra both installed with 0.26.4 of the Hydra Helm chart, which installs Hydra v2.0.1 and Hydra Maester v0.0.25. 0.26.4 is the latest version of the Helm charts, but I was getting the same errors on 0.26.3.
My K8s config for the OAuth2Clients looks like:

apiVersion: hydra.ory.sh/v1alpha1
kind: OAuth2Client
metadata:
  name: oauth2-client-matching-service
spec:
  grantTypes:
    - client_credentials
  scope: serviceToService:*
  metadata:
    serviceName: matching-service
  secretName: oauth2-client-matching-service

@ghost
Copy link

ghost commented Dec 7, 2022

@yasha-spare Do not use Hydra Maester for now. it is clearly out of sync with the Hydra APIs in the latest Helm. Just register your clients manually by running the curl I posted

@yasha-spare
Copy link

yasha-spare commented Dec 7, 2022

Ah, is it actually the case that Hydra Maester just doesn't work with Hydra 2? Can any maintainers comment ... @Demonsthere or @aeneasr maybe?

@ghost
Copy link

ghost commented Dec 7, 2022

Ah, is it actually the case that Hydra Maester just doesn't work with Hydra 2? Can any maintainers comment ... @Demonsthere maybe?

My logs indicated it is pretty clearly the case ... actually had a log saying client_id cannot be set manually

@yasha-spare
Copy link

using default client

Mine suggest that too (see logs above), but I also noticed this log using default client, which is maybe suspicious/indicates misconfiguration or something?

FWIW, I'm happy to hop on a video call to debug together, if that would help for any maintainers looking at this.

@nbrns
Copy link
Author

nbrns commented Dec 7, 2022

Yes, it is no longer supported to set the client ID.
See here: https://www.ory.sh/oauth2-server-openid-ory-hydra-v2#primary-keys

@yasha-spare
Copy link

Yes, it is no longer supported to set the client ID. See here: https://www.ory.sh/oauth2-server-openid-ory-hydra-v2#primary-keys

Yeah, I know with Hydra v2 you can't provide your own client ids, and it does seem that's what Hydra Maester is still trying to do. What's not clear to me is whether or not there's some way to make Hydra Maester work with Hydra v2? Or does it only work with Hydra v1?

@Demonsthere
Copy link
Collaborator

Hello there,
Yes, the maester controller is outdated and we were not able to patch it to support the changes for hydra v2.0 😞. The support for the project is not dropped, but we simply lack the man-power to work on it right now, as the controller needs to be rebuild with the new kubebuilder and for the new api changes.
Sadly, for now I would not recommend using it for hydra v2.0.
My apologies 😞

@aeneasr
Copy link
Member

aeneasr commented Dec 8, 2022

We always accept contritbutions though :) :)

@yasha-spare
Copy link

yasha-spare commented Dec 8, 2022

Cool, ty for confirming, and totally get that there's always a lack of dev resources :)

Spoke with my team today, we considered taking this on and pushing a contribution upstream, but ultimately decided against it - we don't have the dev resources either! Would take us awhile to ramp up as we aren't a Go shop, have no experience with writing K8s CRDs/controllers, no experience with kubebuilder, etc.

If anyone else is curious about our workaround/Hydra Maester alternative:

  • We're using Hydra for secure service-to-service communication
    • i.e. service A gets a Hydra token (client credentials flow) proving it's service A, attaches that token when calling service B, and service B validates it against Hydra
  • We have a lot of services, and a lot of different environments, so manual management of Hydra clients isn't a great option
  • However, we already use ArgoCD, which has these PreSync hooks that run before a service is deployed. So we're just going to write a PreSync job to create Hydra clients and store the id/secret in a K8s secret, if the K8s secret doesn't already exist
    • Ends up similar in practice to the Maester workflow, but should be simpler for our use case

@dan2kx
Copy link

dan2kx commented Jan 7, 2023

As long as the k8 secret doesnt exist already, when you specify the secretName in the CR, hydra will create you a random client_id and client_secret which will then be saved to the k8 secret (named whatever you specified) so it is still possible to use the maestor with v2 as long as you dont mind what the client_id/secret are

@ileixe
Copy link

ileixe commented May 24, 2023

As long as the k8 secret doesnt exist already, when you specify the secretName in the CR, hydra will create you a random client_id and client_secret which will then be saved to the k8 secret (named whatever you specified) so it is still possible to use the maestor with v2 as long as you dont mind what the client_id/secret are

This is quite precious tip. Thanks.

@dpeckett
Copy link
Contributor

dpeckett commented Aug 1, 2023

the controller needs to be rebuild with the new kubebuilder and for the new api changes.

I assume in this context "new kubebuilder" refers to updating kubebuilder (and the controller runtime). And "API changes" refer to updating the Hydra client libs (and perhaps some minor schema changes to the CRs, eg. remove the ability to configure client ids).

If this the above is the case I should be able to quickly bang out the changes (k8s and go is kind of my thing).

Got to say I was quite surprised to see the hydra operator is a community maintained project, I acknowledge the resource constraints Ory corp is under but don't sleep on Kubernetes integration.

@Demonsthere
Copy link
Collaborator

Yeah, i mean we should update kubebuilder to the newest version, and work on refactoring the code into a v1alpha2 or v2alpha1 version of the CRD :).
It's a bit more of a design decision, all ory components are environment agnostic, you can run the binary on windows, bare metal rpi or in the cloud on a k8s cluster as a container. The controllers got created because enough people wanted a more k8s native way to use the applications, but they are nevertheless an extension of the main application and not a core product.
If you'd like to get your hands dirty and start working on it, feel free to reach out to me on our community slack or here :)

@afreakk
Copy link

afreakk commented Feb 13, 2024

Now with v2.2.0 this should work again right?
client_id can again be specified.
ory/hydra#3628

@aeneasr
Copy link
Member

aeneasr commented Feb 13, 2024

Correct

@aeneasr aeneasr closed this as completed Feb 13, 2024
@aeneasr
Copy link
Member

aeneasr commented Feb 13, 2024

Sorry, this is hydra-maester

@aeneasr aeneasr reopened this Feb 13, 2024
@afreakk
Copy link

afreakk commented Mar 5, 2024

Im running hydra-maester v0.0.28 with hydra 2.1.0 and that seems to work fine.
It can create clients with specified clientids.

Hydra-maester 0.29 and above have removed support for this.

@beanow-at-crabnebula
Copy link

With Hydra 2.2.0 we can specify the ID again.

However, if you try this today you will hit this error:

return ctrl.Result{}, fmt.Errorf("oauth2 client %s not found", credentials.ID)

Which explicitly errors when the client ID is set, but not found.
Introduced in #127

Because it assumes setting client ID is not allowed:

It is no longer possible to keep this behavior (due to hydra no longer supporting configurable client ids).

@eadwu
Copy link

eadwu commented Sep 26, 2024

Maester 0.0.28 works fine with 2.2.0 wrt this specific issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working.
Projects
None yet
Development

No branches or pull requests

10 participants