AUTH-440: expand options for the OIDC authenticator #13276

stlaz · 2023-10-24T15:49:18Z

This PR:

allow setting scopes different from openid via options
add the extra scopes and issuer to the config struct
unify both OIDC provider and the OpenShift provider in how the OIDC discovery is queried (continuously in time compared to just during OIDC provider creation)

TODO:

- don't rely on username claims in the token
- if necessary, consume the username from the Self-Subject Review API

openshift-ci-robot · 2023-11-30T12:43:56Z

@stlaz: This pull request references AUTH-440 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set.

In response to this:

This PR:

allow setting scopes different from openid via options

add the extra scopes and issuer to the config struct

unify both OIDC provider and the OpenShift provider in how the OIDC discovery is queried (continuously in time compared to just during OIDC provider creation)

TODO:

- don't rely on username claims in the token
- if necessary, consume the username from the Self-Subject Review API

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

stlaz · 2024-01-02T11:27:36Z

/cherry-pick release-4.15

openshift-cherrypick-robot · 2024-01-02T11:27:38Z

@stlaz: once the present PR merges, I will cherry-pick it on top of release-4.15 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jhadvig

Adding couple of comments. Otherwise the PR looks good 👍
Awesome job @stlaz !!!

jhadvig · 2024-01-03T16:13:46Z

pkg/auth/auth.go

+		return nil, err
+	}
+
+	authConfig := &oidcConfig{


The oidcConfig will be a common config for both auth types, openshift and oidc...right?

pkg/auth/auth.go

jhadvig · 2024-01-03T17:11:41Z

pkg/server/server.go

@@ -285,6 +285,7 @@ func (s *Server) HTTPHandler() http.Handler {
 		handleFunc(authLogoutEndpoint, allowMethod(http.MethodPost, s.handleLogout))
 		handleFunc(AuthLoginCallbackEndpoint, s.Authenticator.CallbackFunc(fn))
 		handle(requestTokenEndpoint, authHandler(s.handleClusterTokenURL))
+		// TODO: only add the following in case the auth type is openshift?


doesn't the external OIDC have an endpoint for invalidating tokens?

IIRC logout URLs exist but are optional

cmd/bridge/config/auth/authoptions.go

openshift-ci-robot · 2024-01-10T11:52:46Z

@stlaz: This pull request references AUTH-440 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to this:

This PR:

allow setting scopes different from openid via options

add the extra scopes and issuer to the config struct

unify both OIDC provider and the OpenShift provider in how the OIDC discovery is queried (continuously in time compared to just during OIDC provider creation)

TODO:

- don't rely on username claims in the token
- if necessary, consume the username from the Self-Subject Review API

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

stlaz · 2024-01-10T13:18:59Z

/test e2e-gcp-console

TheRealJon

/lgtm

openshift-ci · 2024-01-11T16:09:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: stlaz, TheRealJon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~cmd/OWNERS~~ [TheRealJon]
~~pkg/OWNERS~~ [TheRealJon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jhadvig · 2024-01-11T20:20:38Z

QE Approver:
/assign @yapei
Docs Approver:
/assign @opayne1
PX Approver:
/assign @RickJWagner

opayne1 · 2024-01-11T20:55:56Z

/label docs-approved

yanpzhan · 2024-01-12T08:57:58Z

@stlaz Hi, I go through the pr code, but not sure about the test steps though I launched a cluster against the pr. What should I do exactly against the cluster to test the code? Preparing an usable external OIDC ?

yapei · 2024-01-12T09:41:10Z

@yanpzhan is working on this PR

stlaz · 2024-01-12T10:16:41Z

Hi, I go through the pr code, but not sure about the test steps though I launched a cluster against the pr. What should I do exactly against the cluster to test the code? Preparing an usable external OIDC ?

@yanpzhan for now just make sure that the OpenShift authentication is working as it should. We can test the OIDC part along with openshift/console-operator#811 or whenever we're able to get OIDC environment.

I'll start a slack discussion with you on getting it. For now, if OpenShift authentication works I think the PR should be fine to merge.

yanpzhan · 2024-01-15T04:06:13Z

@stlaz Thanks, the test about normal authentication on cluster launched against the pr passed, and covered existing idps cases. I will approve the pr.
/label qe-approved

openshift-ci-robot · 2024-01-15T04:06:19Z

@stlaz: This pull request references AUTH-440 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to this:

This PR:

allow setting scopes different from openid via options

add the extra scopes and issuer to the config struct

unify both OIDC provider and the OpenShift provider in how the OIDC discovery is queried (continuously in time compared to just during OIDC provider creation)

TODO:

- don't rely on username claims in the token
- if necessary, consume the username from the Self-Subject Review API

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

RickJWagner · 2024-01-15T13:24:49Z

/label px-approved

stlaz · 2024-01-15T16:48:20Z

/test all

stlaz · 2024-01-15T23:13:11Z

/test frontend

openshift-ci · 2024-01-16T02:11:06Z

@stlaz: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-cherrypick-robot · 2024-01-16T02:14:04Z

@stlaz: new pull request created: #13509

In response to this:

/cherry-pick release-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-bot · 2024-01-16T05:55:20Z

[ART PR BUILD NOTIFIER]

This PR has been included in build openshift-enterprise-console-container-v4.16.0-202401160407.p0.g8bde63c.assembly.stream for distgit openshift-enterprise-console.
All builds following this will include this PR.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 24, 2023

openshift-ci bot requested review from florkbr and jeff-phillips-18 October 24, 2023 15:51

openshift-ci bot added the component/backend Related to backend label Oct 24, 2023

stlaz force-pushed the direct_oidc_config branch from 61a3e8b to cde9819 Compare October 25, 2023 12:13

stlaz changed the title ~~WIP: expand options in the OIDC authenticator~~ WIP: expand options for the OIDC authenticator Oct 25, 2023

stlaz force-pushed the direct_oidc_config branch 6 times, most recently from 8baa022 to 73f883e Compare November 7, 2023 10:25

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 8, 2023

openshift-ci bot added the component/core Related to console core functionality label Nov 8, 2023

stlaz force-pushed the direct_oidc_config branch from 496c7a8 to a96ce97 Compare November 8, 2023 09:56

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 8, 2023

stlaz force-pushed the direct_oidc_config branch from a96ce97 to 0b0ea5b Compare November 8, 2023 10:02

stlaz force-pushed the direct_oidc_config branch 3 times, most recently from c1598c0 to 8591c45 Compare November 22, 2023 10:13

stlaz changed the title ~~WIP: expand options for the OIDC authenticator~~ AUTH-440: WIP: expand options for the OIDC authenticator Nov 30, 2023

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 30, 2023

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 30, 2023

allow configuring additional OIDC token scopes

65d10e9

stlaz force-pushed the direct_oidc_config branch from 8591c45 to da00eaa Compare December 13, 2023 09:26

stlaz changed the title ~~AUTH-440: WIP: expand options for the OIDC authenticator~~ AUTH-440: expand options for the OIDC authenticator Dec 13, 2023

jhadvig reviewed Jan 3, 2024

View reviewed changes

stlaz force-pushed the direct_oidc_config branch from 6ca44f3 to 32227f8 Compare January 9, 2024 14:31

TheRealJon reviewed Jan 9, 2024

View reviewed changes

cmd/bridge/config/auth/authoptions.go Outdated Show resolved Hide resolved

auth: fail authenticator creation if auth-source is unknown

4f41023

stlaz force-pushed the direct_oidc_config branch from 32227f8 to 4f41023 Compare January 9, 2024 21:21

TheRealJon approved these changes Jan 11, 2024

View reviewed changes

openshift-ci bot assigned TheRealJon Jan 11, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 11, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 11, 2024

openshift-ci bot added the docs-approved Signifies that Docs has signed off on this PR label Jan 11, 2024

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jan 15, 2024

openshift-ci bot added the px-approved Signifies that Product Support has signed off on this PR label Jan 15, 2024

openshift-merge-bot bot merged commit 8bde63c into openshift:master Jan 16, 2024
6 checks passed

openshift-cherrypick-robot mentioned this pull request Jan 16, 2024

[release-4.15] AUTH-440: expand options for the OIDC authenticator #13509

Merged

openshift-ci-robot mentioned this pull request Feb 27, 2024

OCPBUGS-22487: Fix operands list endpoint. #13632

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AUTH-440: expand options for the OIDC authenticator #13276

AUTH-440: expand options for the OIDC authenticator #13276

stlaz commented Oct 24, 2023 •

edited

openshift-ci-robot commented Nov 30, 2023 •

edited by openshift-ci bot

stlaz commented Jan 2, 2024

openshift-cherrypick-robot commented Jan 2, 2024

jhadvig left a comment

jhadvig Jan 3, 2024

stlaz Jan 9, 2024

jhadvig Jan 3, 2024

jhadvig Jan 3, 2024

stlaz Jan 9, 2024

openshift-ci-robot commented Jan 10, 2024 •

edited by openshift-ci bot

stlaz commented Jan 10, 2024

TheRealJon left a comment

openshift-ci bot commented Jan 11, 2024

jhadvig commented Jan 11, 2024

opayne1 commented Jan 11, 2024

yanpzhan commented Jan 12, 2024

yapei commented Jan 12, 2024

stlaz commented Jan 12, 2024 •

edited

yanpzhan commented Jan 15, 2024

openshift-ci-robot commented Jan 15, 2024 •

edited by openshift-ci bot

RickJWagner commented Jan 15, 2024

stlaz commented Jan 15, 2024

stlaz commented Jan 15, 2024

openshift-ci bot commented Jan 16, 2024

openshift-cherrypick-robot commented Jan 16, 2024

openshift-bot commented Jan 16, 2024

AUTH-440: expand options for the OIDC authenticator #13276

AUTH-440: expand options for the OIDC authenticator #13276

Conversation

stlaz commented Oct 24, 2023 • edited

openshift-ci-robot commented Nov 30, 2023 • edited by openshift-ci bot

stlaz commented Jan 2, 2024

openshift-cherrypick-robot commented Jan 2, 2024

jhadvig left a comment

Choose a reason for hiding this comment

jhadvig Jan 3, 2024

Choose a reason for hiding this comment

stlaz Jan 9, 2024

Choose a reason for hiding this comment

jhadvig Jan 3, 2024

Choose a reason for hiding this comment

jhadvig Jan 3, 2024

Choose a reason for hiding this comment

stlaz Jan 9, 2024

Choose a reason for hiding this comment

openshift-ci-robot commented Jan 10, 2024 • edited by openshift-ci bot

stlaz commented Jan 10, 2024

TheRealJon left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jan 11, 2024

jhadvig commented Jan 11, 2024

opayne1 commented Jan 11, 2024

yanpzhan commented Jan 12, 2024

yapei commented Jan 12, 2024

stlaz commented Jan 12, 2024 • edited

yanpzhan commented Jan 15, 2024

openshift-ci-robot commented Jan 15, 2024 • edited by openshift-ci bot

RickJWagner commented Jan 15, 2024

stlaz commented Jan 15, 2024

stlaz commented Jan 15, 2024

openshift-ci bot commented Jan 16, 2024

openshift-cherrypick-robot commented Jan 16, 2024

openshift-bot commented Jan 16, 2024

stlaz commented Oct 24, 2023 •

edited

openshift-ci-robot commented Nov 30, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Jan 10, 2024 •

edited by openshift-ci bot

stlaz commented Jan 12, 2024 •

edited

openshift-ci-robot commented Jan 15, 2024 •

edited by openshift-ci bot