-
Notifications
You must be signed in to change notification settings - Fork 8
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
Github Authentication #52
Conversation
Signed-off-by: Vallari <[email protected]>
1. redirect to teuthology API's login 2. AppBar uses cookie data to show username Signed-off-by: Vallari <[email protected]>
Your Render PR Server URL is https://pulpito-ng-pr-52.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-cjd9ce2vvtos73bueh2g. |
✅ Deploy Preview for pulpito ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Signed-off-by: Vallari <[email protected]>
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.
I haven't carefully read through the diffs on the two PRs yet, but I'm having a problem testing the functionality. I'm testing with docker-compose on a smithi, which of course requires not using localhost
for the various URL values. The pulpito UI loads properly and shows the one scheduled run. When I click the login button, I'm first redirected to -api's /login
, which redirects, to /login/
, which then redirects to /login/None?client_id=None&scope=read:org
src/lib/teuthologyAPI.ts
Outdated
import.meta.env.VITE_TEUTHOLOGY_API || ""; | ||
|
||
function githubLogin() { | ||
window.location.replace(`${TEUTHOLOGY_API_SERVER}/login`); |
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.
If TEUTHOLOGY_API_SERVER
has a trailing slash, the redirect fails
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.
I should be using URL
constructor, right! I'll add a trailing slash to login/ as well.
This could be because of missing |
Signed-off-by: Vallari <[email protected]>
Signed-off-by: Vallari <[email protected]>
This preserves the browser's history stack. Also, add github icon to login button. Signed-off-by: Vallari <[email protected]>
Signed-off-by: Vallari <[email protected]>
Extract cookie related code from Login component to lib/teuthologyAPi.ts Signed-off-by: Vallari <[email protected]>
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.
Checked through this, test it locally as well using docker-compose. Everything works smoothly, so approved and merging!
For local setup,
.env
file:TODO (refactoring):
Profile
to show Username + Logout button in dropdown menu.