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

Send extra parameters with client credentials request #354

Merged
merged 6 commits into from
Mar 15, 2024

Conversation

weirdian2k3
Copy link
Contributor

🔧 Changes

The ExtraParameters property of LoginWithClientCredentials is now included in the request

📚 References

Issue: #353

🔬 Testing

Monitor

		tokenSet, err := authAPI.OAuth.LoginWithClientCredentials(context.Background(), oauth.LoginWithClientCredentialsRequest{
			ClientAuthentication: oauth.ClientAuthentication{
				ClientSecret: clientSecret,
				ClientID:     "test-other-clientid",
			},
			Audience: "test-audience",
			ExtraParameters: map[string]string{
				"test": "value",
			},
		}, oauth.IDTokenValidationOptions{})

Watch the HTTP request to the token endpoint via your favorite network spy

Note that with the fix, &test=value is in the body of the client credentials request

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@weirdian2k3
Copy link
Contributor Author

@ewanharris Hi! Sorry to bother you directly but I saw your name on other PRs. What is required next for me to get this reviewed?

@ewanharris
Copy link
Contributor

@weirdian2k3 my apologies for the lack of review here, let me factor in some time next week to look at this. From a quick check it looks good but I just want to verify the support for this in other SDKs to ensure we're aligned across languages.

ewanharris
ewanharris previously approved these changes Mar 15, 2024
Copy link
Contributor

@ewanharris ewanharris 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 @weirdian2k3! I'll push the test recording separately.

@ewanharris
Copy link
Contributor

Gah I thought I'd be able to merge without the test but I guess not, I made a PR to add it to your branch (seems I can't push) (weirdian2k3#1), if you merge that then I should be able to merge this PR.

Otherwise I'll copy this across to another branch on Monday and then PR it with your commit + the test file commit

@weirdian2k3
Copy link
Contributor Author

@ewanharris I'm sorry for delay getting that merged

To get through the vulnerability check, I had to update the module to use 1.21. I'm not super comfortable bumping that for you, but it seems like all the pipelines are failing until we do so.

@ewanharris
Copy link
Contributor

That's fine @weirdian2k3, the support policy states that it matches the Go support cycle of last 2 majors so I we're fine to bump for the upcoming release. Thanks again for this!

@ewanharris ewanharris enabled auto-merge (squash) March 15, 2024 23:07
@ewanharris ewanharris merged commit dcedb09 into auth0:main Mar 15, 2024
7 checks passed
@sergiught sergiught mentioned this pull request Apr 23, 2024
2 tasks
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