-
Notifications
You must be signed in to change notification settings - Fork 302
Add GitHub App auth provider #15500
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
Add GitHub App auth provider #15500
Conversation
80d5fbe to
bb31446
Compare
|
@eva-vashkevich simplified instructions for creating a new GitHub app can be found at: https://confluence.suse.com/spaces/CU/pages/1903427936/GitHub+App |
8215d27 to
99ac3d1
Compare
eva-vashkevich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
| 2: <li>Click "Generate a new client secret"</li> | ||
| 3: <li>Click "Generate a private key"</li> | ||
| 4: <li>Copy and paste the App ID, Client ID, Client Secret, and Private Key of your newly created OAuth app into the fields below</li> | ||
| 1: <li>Under Client Secrets, click "Generate a new client secret"</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these be in quotes too? And the next two lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for clarity, what are these that you are referencing? I think that the convention I was going for was that targets of actions would be contained within quotation marks - for example, Click "Some label". With this in mind, I think that the targets of Copy & Paste can be contained in quotation marks.
With the current iteration, there's only one line below 521.. I think I've addressed the changes, but let me know if anything is missing.
2d5f111 to
dea6b0f
Compare
eva-vashkevich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
dea6b0f to
e5e8581
Compare
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
Signed-off-by: Phillip Rak <[email protected]>
e5e8581 to
40b5058
Compare
|
This depends on rancher/rancher#51686 |
|
Backend now merged so this can now do the same once CI passes. |
|
Tested with latest backend. everything seems to work fine |
Summary
This adds a new GitHub App auth provider.
Fixes #14796
Occurred changes and/or fixed issues
Technical notes summary
Similar to other auth providers, this change utilizes existing functionality to allow the GitHub App auth provider to function - meaning that the GitHub auth provider has been modified to support this feature in order to cut down on duplication.
There are two new "steps" components, which render the instructions for the github auth providers. These can be distilled down into a single component, driven by data to properly render contents, but I thought the two component approach to be more straightforward for now.
Areas or cases that should be tested
Areas which could experience regressions
Screenshot/Video
Checklist