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

Implement configurable OIDC claims extraction #44

Merged
merged 2 commits into from
Jul 4, 2024

Conversation

ErmakovDmitriy
Copy link
Contributor

This PR expands the authentication agent with methods to extract an OAuth2 token claims and set
their values to HAProxy session variables.

The token claims are prefixed with "token_claim_" and can be used as in an example below:

http-request set-header X-OIDC-Username %[var(sess.auth.token_claim_name)] if acl_app3 authenticated

#16

@mougams
Copy link
Contributor

mougams commented Jun 17, 2024

Hi @ErmakovDmitriy - sorry for the delay, we missed this PR. Could you please fix conflicts and push back your patch? Thx

@mougams
Copy link
Contributor

mougams commented Jun 24, 2024

Hi @ErmakovDmitriy - sorry for the delay, we missed this PR. Could you please fix conflicts and push back your patch? Thx

@ErmakovDmitriy I still see out-of-date branch -> could you please rebase and push? Thx

@ErmakovDmitriy
Copy link
Contributor Author

Hi @mougams,

Thank you for your patience!
Please, take a look at the current state. Sorry again for me being clumsy with git.

Copy link
Contributor

@mougams mougams left a comment

Choose a reason for hiding this comment

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

Minor comment - other than that, looks good

And thanks for having updated the test

return false, []action.Action{BuildRedirectURLMessage(authorizationURL)}, nil
}

func (oa *OIDCAuthenticator) builaAuthorizationURL(domain string, oauthArgs OAuthArgs) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: rename to buildAuthorizationURL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for noticing it!

Copy link
Contributor

Choose a reason for hiding this comment

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

The tests don't pass, it just hang. Already retriggered twice, same result.

Retrying another time 🤞

Copy link
Contributor

Choose a reason for hiding this comment

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

Test passed - I wonder if this is not linked to the HAProxy version - running in the CI, I see version 2.9.9 - and when running locally, I get v2.9.7.

Anyway, not linked to this patch - I'm merging it.

@mougams mougams merged commit 8f05dde into criteo:master Jul 4, 2024
1 check passed
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