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

authenticate: fix identity provider id in encrypted query string #4006

Merged
merged 2 commits into from Feb 23, 2023

Conversation

calebdoxsey
Copy link
Contributor

Summary

Per-route IDP credentials use a pomerium_idp_id query string parameter to indicate which credentials they need. The current code is not working because the pomerium_idp_id is inside an HPKE encrypted query string. This updates the code to decrypted the URL parameters to find it.

Related issues

Fixes #3976

Checklist

  • reference any related issues
  • updated unit tests
  • add appropriate tag (improvement / bug / etc)
  • ready for review

@calebdoxsey calebdoxsey added bug Something isn't working backport 0-21-0 labels Feb 22, 2023
@calebdoxsey calebdoxsey requested a review from a team as a code owner February 22, 2023 20:11
@coveralls
Copy link

coveralls commented Feb 22, 2023

Coverage Status

Coverage: 63.243% (+0.09%) from 63.158% when pulling 96bdff5 on cdoxsey/per-route-idp-credentials into df54a0c on main.

Comment on lines +607 to +609
if idpID == "" {
idpID = vs.Get(urlutil.QueryIdentityProviderID)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why this clause is still required if parameter is inside hpke query?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we pass the identity provider id from userinfo endpoints? I'm not totally sure. I kept it because I was afraid I'd break something by removing it.

@calebdoxsey calebdoxsey merged commit 00c047b into main Feb 23, 2023
@calebdoxsey calebdoxsey deleted the cdoxsey/per-route-idp-credentials branch February 23, 2023 15:30
backport-actions-token bot pushed a commit that referenced this pull request Feb 23, 2023
Co-authored-by: Denis Mishin <dmishin@pomerium.com>
calebdoxsey added a commit that referenced this pull request Feb 23, 2023
authenticate: fix identity provider id in encrypted query string (#4006)

Co-authored-by: Caleb Doxsey <cdoxsey@pomerium.com>
Co-authored-by: Denis Mishin <dmishin@pomerium.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 0-21-0 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strange behavior with setting an identity provider client id per route
3 participants