singingwolfboy / flask-dance

Doing the OAuth dance with style using Flask, requests, and oauthlib.
https://pypi.python.org/pypi/Flask-Dance/
MIT License
997 stars 156 forks source link

Add ORCID OAuth2 provider #403

Closed ml-evs closed 1 year ago

ml-evs commented 1 year ago

This PR adds support for ORCID (https://orcid.org/) and its sandbox server (https://sandbox.orcid.org) via a blueprint init option use_orcid_sandbox.

It is slightly tricky to test this as OAuth configs at ORCID require human intervention at ORCID's end, and even then, the ORCID sandbox does not allow non-HTTPS callback URIs (like localhost). I was able to use this code to authorize via the ORCID sandbox by monkey-patching the authorized URL in the base blueprint to point at Google's OAuth playground using the openid scope (might be useful if this was easier to do).

Thanks for flask-dance!

codecov[bot] commented 1 year ago

Codecov Report

Merging #403 (41d6436) into main (1e316b1) will increase coverage by 0.11%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #403      +/-   ##
==========================================
+ Coverage   93.30%   93.41%   +0.11%     
==========================================
  Files          36       36              
  Lines        1030     1048      +18     
==========================================
+ Hits          961      979      +18     
  Misses         69       69              
Impacted Files Coverage Δ
flask_dance/contrib/orcid.py 100.00% <100.00%> (ø)
flask_dance/__init__.py

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

ml-evs commented 1 year ago

Thanks for the review, rushed this out a bit and should have left it as a draft until I could finish it off. I should have addressed all your concerns (apologies if you had to manually authorize each rerun of the workflows!)

singingwolfboy commented 1 year ago

Looks great, thank you! I'll cut a new release in the next few days.

ml-evs commented 1 year ago

Looks great, thank you! I'll cut a new release in the next few days.

Thanks, I appreciate it!

singingwolfboy commented 1 year ago

@ml-evs I just published version 6.2.0 a few hours ago, including your change! Thanks again for contributing to this project. 😄

ml-evs commented 1 year ago

Awesome, thanks for letting me know.

mpound commented 1 year ago

@ml-evs Do you have a working example using orcid for authentication you could share? similar to the github example on https://flask-dance.readthedocs.io/en/latest/ ? I tried the obvious substitutions of 'orcid' for 'github' in the python script, but get weird errors like "Incorrect OAuth Link for client id , missing scope parameter." However changing the call to explicitily set the scope: make_orcid_blueprint(scope=["authenticate"]) results in a different error "CSRF Warning! State not equal in request and response." A working example might help clarify what I am doing wrong. thanks.

ml-evs commented 1 year ago

Hi @mpound, sure thing, I can type something up when I get home. Have you registered an app with the sandbox ORCID API already? For me it was process with manual approval from the ORCID team.

Just in case it is a simple issue, I can see that in my code (unfortunately closed source for now) my scope is set to "/authenticate" rather than the list ["authenticate"]. I am also passing sandbox=True to my blueprint.

mpound commented 1 year ago

ok, I got a little farther with your hints. It appears to authenticate correctly at resp = orcid.get("/user") assert resp.ok

But resp.ok is False. I did register the test app with sandbox orcid on the developer tools section of my account, which gave me the CLIENT_ID and CLIENT_SECRET.