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.

Sign up for GitHub

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

Jump to bottom

Adding support for OIDCProviderTokenEndpoint, OIDCOAuthIntrospectionEndpoint and OIDCScope #747

Merged
merged 7 commits into from
Feb 23, 2020
Merged

Adding support for OIDCProviderTokenEndpoint, OIDCOAuthIntrospectionEndpoint and OIDCScope #747

merged 7 commits into from
Feb 23, 2020

Conversation

abellotti
Copy link
Member

@abellotti abellotti commented Feb 21, 2020 •
edited
Loading

Adding support for OIDCProviderTokenEndpoint, OIDCOAuthIntrospectionEndpoint and OIDCScope

  • Adding support for OIDCProviderTokenEndpoint and OIDCOAuthIntrospectionEndpoint
    if defined before resorting to the related token_endpoint and token_introspection_endpoint
    keys returned by the provider metadata URL.

  • Also specifying the scope for the provider calls to get and verify jwt tokens if defined
    in the OIDCScope parameter

@abellotti
Copy link
Member Author

/cc @Fryguy @gtanzillo just the code, about to test now.

@Fryguy
Copy link
Member

Fryguy commented Feb 21, 2020

Code-wise LGTM (aside from the one comment).

@abellotti
Adding support for OIDCProviderTokenEndpoint, OIDCOAuthIntrospectionE…
f39454b 
…ndpoint and OIDCScope

- Adding support for OIDCProviderTokenEndpoint and OIDCOAuthIntrospectionEndpoint
  if defined before resorting to the related token_endpoint and token_introspection_endpoint
  keys returned by the provider metadata URL.

- Also specifying the scope for the provider calls to get and verify jwt tokens if defined
  in the OIDCScope parameter
@abellotti abellotti force-pushed the support_optional_oidc_parameters branch from 26c74ec to f39454b Compare February 21, 2020 21:43
gtanzillo
gtanzillo approved these changes Feb 21, 2020
View reviewed changes
Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@abellotti
Was missing the use_ssl when fetching the provider metadata, as that …
2fcb443 
…was not

using the post_form which handles ssl by default.
@abellotti
Updated parameter parsing so we skip over commented lines, i.e.
9c00fda 
if the parameter specified does not start the line.
@abellotti
Preferring to use basic auth for client id/secret authentication
2273eb4 
against the provider for getting tokens and introspecting them.
@abellotti
was using the wrong claim for the username, should have been preferre…
a04015c 
…d_username

as declared in manageiq-remote-user-openidc.conf.
@miq-bot
Copy link
Member

miq-bot commented Feb 22, 2020

Checked commits abellotti/manageiq-api@f39454b~...a04015c with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.20.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@abellotti
Copy link
Member Author

Was tested successfully with both Keycloak and IAM Identity Providers, un-wipping.

@abellotti abellotti changed the title [WIP] Adding support for OIDCProviderTokenEndpoint, OIDCOAuthIntrospectionEndpoint and OIDCScope Adding support for OIDCProviderTokenEndpoint, OIDCOAuthIntrospectionEndpoint and OIDCScope Feb 23, 2020
@abellotti abellotti removed the wip label Feb 23, 2020
@chessbyte chessbyte assigned chessbyte and unassigned Fryguy Feb 23, 2020
@chessbyte chessbyte merged commit 7677117 into ManageIQ:master Feb 23, 2020
@chessbyte chessbyte added this to the Sprint 131 Ending Mar 2, 2020 milestone Feb 23, 2020
@abellotti abellotti mentioned this pull request Feb 23, 2020
Open
simaishi pushed a commit that referenced this pull request Feb 24, 2020
@chessbyte @simaishi
Merge pull request #747 from abellotti/support_optional_oidc_parameters
32158db 
Adding support for OIDCProviderTokenEndpoint, OIDCOAuthIntrospectionEndpoint and OIDCScope

(cherry picked from commit 7677117)

https://bugzilla.redhat.com/show_bug.cgi?id=1805914
@simaishi
Copy link
Contributor

Ivanchuk backport details:

$ git log -1
commit 32158db30eaeb3766a747198c0f67f8b725d7f59
Author: Oleg Barenboim <chessbyte@gmail.com>
Date:   Sun Feb 23 07:51:37 2020 -0500

    Merge pull request #747 from abellotti/support_optional_oidc_parameters

    Adding support for OIDCProviderTokenEndpoint, OIDCOAuthIntrospectionEndpoint and OIDCScope

    (cherry picked from commit 7677117443d76c0a6aecdb0bfaa1030183cea808)

    https://bugzilla.redhat.com/show_bug.cgi?id=1805914

@chessbyte
Copy link
Member

Part of ManageIQ/manageiq#19867

@chessbyte chessbyte mentioned this pull request Feb 24, 2020
Closed
16 tasks
@chessbyte
Copy link
Member

This PR builds on top of #737

This was referenced Mar 6, 2020
Merged
Merged
@abellotti abellotti mentioned this pull request Mar 19, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fryguy Fryguy Fryguy left review comments

@gtanzillo gtanzillo gtanzillo approved these changes

Assignees

@chessbyte chessbyte

Labels
enhancement ivanchuk/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@abellotti @Fryguy @miq-bot @simaishi @chessbyte @gtanzillo