Skip to content

fix: override oauth2.HTTPClient w/ ctx correctly #4006

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

Closed

Conversation

lol768
Copy link
Contributor

@lol768 lol768 commented Jun 28, 2025

Previously, the various commands that exist to help you do the OAuth2 flow locally were not overriding the oauth2.HTTPClient via the context correctly. This meant that if you were e.g. running Hydra locally and using a self-signed/test TLS certificate, the flows would fail with TLS errors even in the presence of the --skip-tls-verify flag.

Steps to reproduce bug:

  1. Set SERVE_TLS_CERT_BASE64 and SERVE_TLS_KEY_BASE64 to something self-signed/PEM that will otherwise not be considered valid.

  2. Enable TLS serving in Hydra's config:

    serve:
    +  tls:
    +    enabled: true
  3. Create a test client:

    $ hydra create client --skip-tls-verify \
        --name demo-client \
        --secret password123 \
        --grant-type authorization_code,refresh_token \
        --response-type token,code,id_token \
        --scope openid,offline,example.scope \
        --redirect-uri http://127.0.0.1:9010/callback
    
  4. Use perform to try and do an authorization-code:

    $ hydra perform --skip-tls-verify authorization-code --port 9010 --client-id 387069d9-9a01-4e53-b72c-507a59f0f4df --client-secret password123 --scope openid,offline,example.scope --skip-tls-verify --auth-url https://127.0.0.1:4444/oauth2/auth
  5. Observe failure, as if --skip-tls-verify had not been set at all:
    image

  6. Apply diff in this PR.

  7. It works as you'd expect:
    image

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got the approval (please contact
    [email protected]) from the maintainers to push
    the changes.
  • I have added or changed the documentation. Not applicable, docs already contained information about this flag; it just didn't work.

Further Comments

Minor change.

@lol768 lol768 requested review from aeneasr and a team as code owners June 28, 2025 17:10
@lol768 lol768 changed the title fix(cmd): override oauth2.HTTPClient w/ ctx correctly fix: override oauth2.HTTPClient w/ ctx correctly Jun 28, 2025
Copy link
Contributor

@alnr alnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

We're having some issues with our CI since our move to an internal monorepo. We'll merge this change after that's resolved.

@alnr alnr force-pushed the fix/override-oauth2-httpclient-properly branch from 18c79a0 to 3834fab Compare July 2, 2025 14:13
@alnr alnr closed this Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants