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.

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

Implement automated authorization flow on API page #423

Closed
shreddd opened this issue Dec 20, 2023 · 18 comments
Closed

Implement automated authorization flow on API page #423

shreddd opened this issue Dec 20, 2023 · 18 comments
Assignees

Comments

@shreddd
Copy link
Collaborator

shreddd commented Dec 20, 2023

It would be useful to be able to click through the authorize button, so that the orcid token automatically gets pulled into the users session, rather than having to manually enter the credentials in the client ID field.

@dwinston
Copy link
Collaborator

set orcid redirect to login_for_access_token route?

@dwinston
Copy link
Collaborator

dwinston commented Jan 4, 2024

set orcid redirect to login_for_access_token route?

I don't think this would simply work how we want it to work, because I think the Swagger UI's POST to the login_for_access_token route is an AJAX request that populates a JavaScript variable on the already-loaded DOM.

I think we'll need to modify the Swagger UI payload ("hydrate" it, if you will) to e.g. look for a set cookie and to use the cookie to feed it's front-end authorization process.

@dwinston
Copy link
Collaborator

dwinston commented Jan 4, 2024

From https://fastapi.tiangolo.com/how-to/configure-swagger-ui/, it seems that reconfiguring the get_swagger_ui_html function (or modifying it, but hopefully not) might be the way to go here.

@dwinston
Copy link
Collaborator

dwinston commented Jan 4, 2024

@cferdinandi can you look into a solution for this? It amounts to implementing a generic way for Swagger UI's HTML response (a generated Swagger UI page for a dev instance of this project to which a GitHub Action continuously deploys the main branch is here) to look for a cookie on load in order to Authenticate that user. The cookie would be set by the backend when delivering Swagger UI's HTML response.

@shreddd
Copy link
Collaborator Author

shreddd commented Jan 4, 2024

Should this be consolidated with #428 ?

@cferdinandi
Copy link

@dwinston @shreddd Just jumping into this project... what do I need to do to replicate/test/spin-up this environment?

@dwinston
Copy link
Collaborator

dwinston commented Jan 5, 2024

@cferdinandi I sent you an invite to my "NMDC Dev" org on gitpod.io (this repo has .gitpod.* init files). I think this will be the quickest way to dive in.

@dwinston
Copy link
Collaborator

dwinston commented Jan 5, 2024

@shreddd just sent you an invite too for good measure, in case you're curious (but the resource requirements are well within their free plan).

@cferdinandi
Copy link

Thanks @dwinston , got it! Not really sure what to do next, but I assume more details will follow!

@dwinston
Copy link
Collaborator

dwinston commented Jan 8, 2024

@cferdinandi I think I want https://swagger.io/docs/specification/authentication/cookie-authentication/ , which is not implemented in Swagger UI because swagger-api/swagger-js#1163 , but our case fits with this comment (swagger-api/swagger-js#1163 (comment)), so I'm hoping you can prepare a new javascript file to pass to fastapi's get_swagger_ui_html function as its swagger_js_url parameter.

so strictly speaking, you do not need to replicate/spin-up the nmdc-runtime environment -- you can set up a toy e.g. nodejs project for this.

@cferdinandi
Copy link

@dwinston Gotcha, so this is a generic component, decoupled from the specific project at the moment? If so, I'm on it!

@cferdinandi
Copy link

@dwinston Ok, looking into this a bit more... I think I'm missing some context.

The general task is straightforward: get cookie, send with request. But to implement this in the context in which you're using it, it would be immensely helpful to have an actual working environment to start with and some details on how to actually replicate the current state.

From re-reading this a few times, it seems like there's some specifics to your setup that matter. Calling the Fast API, how the auth token is actually returned, if it's currently set in some way or not, whether or not JS can access it, etc.

How can we get a working environment setup?

@cferdinandi
Copy link

cferdinandi commented Jan 11, 2024

@dwinston it looks like setting withCredentials to true should do the trick so long as the cookie was set on the same domain. Was/is it?

I tried searching the code base for a SwaggerUI config but couldn’t find anything.

More details here: https://swagger.io/docs/open-source-tools/swagger-ui/usage/configuration/

dwinston added a commit that referenced this issue Jan 11, 2024
@dwinston
Copy link
Collaborator

@cferdinandi this looks very promising. And here I was looking to save 1 hour of reading documentation with 4 hours of coding. 🙂

So it could be sufficient to:

  1. keep the orcid redirect url to /orcid_token endpoint
  2. set a cookie in the response
  3. redirect to the docs page with withCredentials true in the swagger config (and true in general)
  4. ensure the normal get_current_user path dependency function looks for this cookie.

And thus, after successful orcid auth, a user should see a refresh of the swagger UI where it has recognized authentication via the same-origin cookie.

@PeopleMakeCulture go ahead and take a crack at this.

@dwinston
Copy link
Collaborator

@cferdinandi
Copy link

@dwinston Sorry it took me a hot minute to grok what you were asking. I was hung up on the auth piece and how the cookie got there in the first place!

@PeopleMakeCulture
Copy link
Collaborator

PeopleMakeCulture commented Jan 20, 2024

@dwinston I looked over your PR and removed some unnecessary code. Other than that LGTM

dwinston added a commit that referenced this issue Jan 22, 2024
* feat: login-with-orcid link at top

for #423`

* new GH action to lint and reformat

* commit and push reformatting

closes #438

* fix

* quicken lint GH action

* test: for #439

* fix: author-ize

* fix: quote

* fix: autosetup remote

* fix: ensure HEAD ref for git push

* try .sha

* fix: arg sent to wrong step

* style: reformat

* inprogress: do not merge

* inprogress: do not merge

* inprogress: do not merge: add todo

* [do not merge] login w/o logout

* remove old orcid endpoints

* remove auth-action tags

* remove commented out orcid_cookie_test

* clean: abandon auth-action hack for now

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Jing Cao <jingcao.me@gmail.com>
@dwinston
Copy link
Collaborator

addressed imperfectly via #441

  • when logged in, one still sees "Login with ORCiD" in the static docs, but you can see that lock icons are locked.

dwinston added a commit that referenced this issue Feb 2, 2024
* feat: login-with-orcid link at top

for #423`

* new GH action to lint and reformat

* commit and push reformatting

closes #438

* fix

* quicken lint GH action

* test: for #439

* fix: author-ize

* fix: quote

* fix: autosetup remote

* fix: ensure HEAD ref for git push

* try .sha

* fix: arg sent to wrong step

* style: reformat

* inprogress: do not merge

* inprogress: do not merge

* inprogress: do not merge: add todo

* [do not merge] login w/o logout

* remove old orcid endpoints

* remove auth-action tags

* remove commented out orcid_cookie_test

* clean: abandon auth-action hack for now

---------

Co-authored-by: github-actions <github-actions@github.com>
Co-authored-by: Jing Cao <jingcao.me@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Status: Scored
Development

No branches or pull requests

4 participants