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

AUTH-355: add support for oauth authorization code flow #1531

Merged
merged 1 commit into from Jun 20, 2023
Merged

AUTH-355: add support for oauth authorization code flow #1531

merged 1 commit into from Jun 20, 2023

Conversation

liouk
Copy link
Member

@liouk liouk commented May 17, 2023

This PR adds support for the OAuth Authorization Code Flow. This was before part of openshift/oc and in particular pkg/helpers/tokencmd/request_token.go.

This refactoring is required for openshift/oc#1402 to be rebased, due to the merge of openshift/oc#991.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 17, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented May 17, 2023 •
edited by openshift-ci bot

@liouk: This pull request references AUTH-356 which is a valid jira issue.

In response to this:

This PR adds support for the OAuth Authorization Code Flow. This was before part of openshift/oc and in particular pkg/helpers/tokencmd/request_token.go.

This refactoring is required for openshift/oc#1402 to be rebased, due to the merge of openshift/oc#991.

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-ci 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 May 17, 2023
@openshift-ci openshift-ci bot requested review from ibihim and stlaz May 17, 2023 10:11
@liouk liouk mentioned this pull request May 17, 2023
Merged
stlaz
stlaz reviewed May 18, 2023
View reviewed changes
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
@liouk liouk force-pushed the authz-code-pkce branch 2 times, most recently from b80a11c to 81eaca5 Compare May 23, 2023 11:01
@liouk
Copy link
Member Author

liouk commented May 23, 2023

/retest-required

@liouk liouk changed the title WIP: AUTH-356: add support for oauth authorization code flow AUTH-356: add support for oauth authorization code flow May 23, 2023
@openshift-ci 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 May 23, 2023
@liouk liouk changed the title AUTH-356: add support for oauth authorization code flow AUTH-355: add support for oauth authorization code flow May 23, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented May 23, 2023 •
edited by openshift-ci bot

@liouk: This pull request references AUTH-355 which is a valid jira issue.

In response to this:

This PR adds support for the OAuth Authorization Code Flow. This was before part of openshift/oc and in particular pkg/helpers/tokencmd/request_token.go.

This refactoring is required for openshift/oc#1402 to be rebased, due to the merge of openshift/oc#991.

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.

@liouk liouk requested a review from stlaz May 24, 2023 12:41
stlaz
stlaz reviewed May 26, 2023
View reviewed changes
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
@liouk
Copy link
Member Author

liouk commented May 26, 2023

/hold
contains fixups

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 26, 2023
@liouk liouk requested a review from stlaz May 26, 2023 15:49
stlaz
stlaz reviewed May 30, 2023
View reviewed changes
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
pkg/oauth/tokenrequest/request_token.go Outdated Show resolved Hide resolved
@liouk liouk requested a review from stlaz May 31, 2023 12:57
@stlaz
Copy link
Member

stlaz commented Jun 1, 2023

Looks good. Could you try to vendor it in your openshift/oc#1402 to get a sort of a proof PR?

@liouk
Copy link
Member Author

liouk commented Jun 1, 2023

Looks good. Could you try to vendor it in your openshift/oc#1402 to get a sort of a proof PR?

Good idea 👍 -- I already have done this locally for my tests, so I'll tidy it up a bit and will push it to openshift/oc#1402

liouk added a commit to liouk/oc that referenced this pull request Jun 1, 2023
@liouk
DO NOT MERGE: pretend vendor update of library-go dep
f6df017 
This commit brings in the changes done in openshift/library-go#1531
@liouk
Copy link
Member Author

liouk commented Jun 20, 2023

/hold cancel
fixups squashed

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 20, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 20, 2023

@liouk: 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.

@stlaz
Copy link
Member

stlaz commented Jun 20, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 20, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 20, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 20, 2023
@openshift-merge-robot openshift-merge-robot merged commit 504ca4b into openshift:master Jun 20, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ibihim ibihim Awaiting requested review from ibihim

@stlaz stlaz Awaiting requested review from stlaz

Assignees

@stlaz stlaz

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@liouk @openshift-ci-robot @stlaz @openshift-merge-robot