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

Allow OAuth scopes in discovery document #5

Merged
merged 2 commits into from
Jul 29, 2020

Conversation

jceresini
Copy link
Contributor

Following through after a feature request here: hashicorp/terraform#25354

This adds support for reading a list of scopes from the discovery document for an OAuth2 configuration. Scopes are optional for authz requests in OAuth2, but required for OIDC.

I added the tests requested in the linked feature request, however 2 of the existing tests fail for me with golang version 1.14.4. I saw an open PR to fix that already (#4), so I left it as-is in this PR

@hashicorp-cla
Copy link

hashicorp-cla commented Jul 9, 2020

CLA assistant check
All committers have signed the CLA.

@jceresini
Copy link
Contributor Author

How does my employer go about signing a corporate CLA?

  1. You represent that you are legally entitled to grant the above license. If your employer(s) has rights to intellectual property that you create that includes your Contributions, you represent that you have received permission to make Contributions on behalf of that employer, that you will have received permission from your current and future employers for all future Contributions, that your applicable employer has waived such rights for all of your current and future Contributions to HashiCorp, or that your employer has executed a separate Corporate CLA with HashiCorp.

apparentlymart and others added 2 commits July 28, 2020 17:17
The net/url package in the standard library is now producing slightly
different error messages than before.
Support for scopes is required for OAuth2 server
implementations that are following the additional
requirements imposed by the OpenID Connect
specification.
@apparentlymart
Copy link
Contributor

Thanks @jceresini! This looks great to me. Thanks for working on it!

While I was testing I also saw the failures relating to existing error messages include text from the Go standard library that has drifted in recent releases, so I added an extra commit here to address that so we can see for certain that all of the tests remain passing after your work.

I'm going to merge both of these commits now. I think the next step for your original request hashicorp/terraform#25354 would be to update Terraform itself to use a newer version of this library that includes these changes and then teach the terraform login support code how to handle Scopes as we were discussing over there.

The main Terraform codebase is currently in v0.13 release freeze, so we won't be able to merge this new feature in Terraform itself until the main branch reopens for v0.14 development in a few weeks, but I don't expect anything else related to terraform login to change in the meantime so if you're interested in working on it it should be safe to work against the current code on the main branch and in the unlikely event that some other conflicting changes do show up in the meantime we should be able to resolve the conflicts as part of reviewing it.

Thanks again!

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