Skip to content

Conversation

RyoKusnadi
Copy link

Refer To Issue 473
Changes:

  • Add RequirePKCE(ctx, issuerURL string, c *http.Client) error to check for PKCE support via server metadata and return an error if not supported.

@samthanawalla samthanawalla self-requested a review October 7, 2025 16:37
Copy link
Contributor

@samthanawalla samthanawalla left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
I moved some stuff around so you will have to rebase!

// RequirePKCE checks that the authorization server for issuerURL supports PKCE,
// by verifying that CodeChallengeMethodsSupported is non-empty.
// It returns an error if metadata cannot be fetched or PKCE is not advertised.
func RequirePKCE(ctx context.Context, issuerURL string, c *http.Client) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

move this to auth_meta after rebasing

@RyoKusnadi RyoKusnadi force-pushed the ryo/check-pcke-support branch from 6d6e1c2 to 40f7fb4 Compare October 8, 2025 13:40
@RyoKusnadi
Copy link
Author

RyoKusnadi commented Oct 8, 2025

Hi @samthanawalla updated, kindly help to review again. thanks!

return nil, fmt.Errorf("failed to get auth server metadata from %q: %w", issuerURL, errors.Join(errs...))
}

// RequirePKCE checks that the authorization server for issuerURL supports PKCE,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should have an exported function for this. Why can't we do it inside GetAuthServerMeta?

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.

3 participants