Reimplement Authentication #94

chelseatroy · 2020-03-25T22:01:46Z

Authentication was originally implemented in this pull request: #92

Ultimately we settled on this solution instead:
#92 (comment)

This is the implementation of that solution.

A few additional changes went into this PR:

Theia previously didn't have any built-in environment variable management. She now has a .env file and doesn't rely on people putting variables in their bash profile.
The tests work regardless of where theia lives on your local computer. This did not used to be true.
Theia's homepage and login pages have a small amount of styling. It is quite ugly, but it serves two important purposes: 1) I look at these pages a lot and it makes me smile 2) ugly styling serves as a subtle reminder that we need to consider how this should, one day, look for the researchers.

ALSO:
Prior to this, we had a way that authentication to Panoptes is canonically done in our client applications, but I am not aware of documentation of this process, and not all of our client apps follow it (Caesar, for example). So I put together a rudimentary list of how we do this, which I can beef up and stick into the Panoptes documentation, if it pleases the judge.

https://github.com/zooniverse/theia/wiki/How-to-Set-Up-a-Client-App-to-Authenticate-to-Panoptes

1. User signs in with social auth 2. User makes a request for data to a specific project 3. Theia verifies that user has upload rights for that project 4. Theia performs pipeline and uses her own credentials to upload to the project In order for this to work, the theia user (current username 'therealtheia') must be added as a collaborator to any project that wishes to use one of her pipelines.

+ Instead of fetching every project that theia can READ (which includes all public projects), this page now only fetches projects to which theia can WRITE (collaborator role). That is a lot fewer projects, since there are SO MANY public projects.

+ Mostly for my own amusement. I spend a lot of time looking at these pages, so I turned them into something that makes me smile

+ Previously they checked hardcoded paths to individual files. Now they check relative paths.

…file names.

camallen

LGTM

camallen · 2020-03-26T15:49:17Z

.env.example

+export USGS_USERNAME=something_secret_goes_here
+export USGS_PASSWORD=something_secret_goes_here
+
+export PANOPTES_PROD_CLIENT_ID=something_secret_goes_here


Can these just be collapsed to non deployed environment keys and allow the deploying environment to inject the correct value? E.g. PANOPTES_PROD_CLIENT_ID becomes PANOPTES_CLIENT_ID where the value is specified differently in staging & production

Hm; well, this is sort of an interim step.

When I inherited theia, she could only connect to production version of panoptes. Not staging. I would like to get to the point where she can also upload subjects to staging for testing out changes without affecting data on prod, and I think we're almost there.

In the meantime, I have this set up so that I can switch between pointing at projects on prod Panoptes and staging Panoptes with my local theia, and not have to keep copy and pasting over the keys as I am developing.

For deployment, we can collapse them.

camallen · 2020-03-26T15:49:38Z

conftest.py

@@ -0,0 +1 @@
+


is this file required?

It is; without it, pytest is unable to locate the tests unless they are at a very specific file path; it's apparently a known thing, and this is the workaround.

camallen · 2020-03-26T15:52:40Z

theia/api/templates/registration/login.html

-  <h2>Login</h2>
-  <a href="{% url 'social:begin' 'panoptes' %}">Login with Panoptes</a><br>
+  <h2>Hi. We're glad you're here.</h2>
+  <a href="{% url 'social:begin' 'panoptes' %}" class="theia-button">Login with Panoptes</a><br>


Can we use the Zooniverse term instead of panoptes. Panoptes is really an internal codename but it's just the Zooniverse API that supports authentication as well. We could slice authentication out of panoptes to it's own service name long term.

Suggested change

<a href="{% url 'social:begin' 'panoptes' %}" class="theia-button">Login with Panoptes</a><br>

<a href="{% url 'social:begin' 'zooniverse' %}" class="theia-button">Login with Zooniverse</a><br>

zooniverse/panoptes-cli#94

camallen · 2020-03-26T16:00:13Z

theia/api/views/imagery_request_view_set.py

+        with get_authenticated_panoptes(
+                request.session['bearer_token'],
+                request.session['refresh_token'],


Not blocking - i don't think this user will have a refresh token in the session if you used the oauth implicit flow to get bearer tokens

Good point; and we don't use it at the moment either

camallen · 2020-03-26T16:01:06Z

theia/api/views/imagery_request_view_set.py

+
+def get_authenticated_panoptes(bearer_token, refresh_token, bearer_expiry):
+    authenticated_panoptes = Panoptes(


Maybe clarify that this is using the supplied user/client credentials rather than server side ones

Suggested change

authenticated_panoptes = Panoptes(

user_authenticated_panoptes = Panoptes(

camallen · 2020-03-26T16:01:48Z

theia/operations/panoptes_operations/upload_subject.py

-             endpoint=PanoptesUtils.base_url(),
-             client_id=PanoptesUtils.client_id(),
-             client_secret=PanoptesUtils.client_secret()
+        authenticated_client = Panoptes(


maybe clarify this is using the special server side oauth client app vs user supplied one

Suggested change

authenticated_client = Panoptes(

server_authenticated_client = Panoptes(

…ggestions

chelseatroy added 7 commits March 25, 2020 10:12

Add dotenv file to .gitignore

eb267d1

Include an example dotenv file

c26b9c9

Faster login page load

311fae0

+ Instead of fetching every project that theia can READ (which includes all public projects), this page now only fetches projects to which theia can WRITE (collaborator role). That is a lot fewer projects, since there are SO MANY public projects.

Dress up the UI for the login and logout pages a little

901c6c0

+ Mostly for my own amusement. I spend a lot of time looking at these pages, so I turned them into something that makes me smile

Make the tests more robust

68f4ac3

+ Previously they checked hardcoded paths to individual files. Now they check relative paths.

Upload tests need to mock opening files if they're going to use fake …

2cf3309

…file names.

chelseatroy requested review from camallen and zwolf March 25, 2020 22:02

camallen approved these changes Mar 26, 2020

View reviewed changes

Clean up some variables and make their names more clear, per Cam's su…

1e2b714

…ggestions

chelseatroy merged commit 44589a2 into master Mar 26, 2020

yuenmichelle1 deleted the reimplement-authentication branch April 12, 2024 19:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reimplement Authentication #94

Reimplement Authentication #94

chelseatroy commented Mar 25, 2020

camallen left a comment

camallen Mar 26, 2020

chelseatroy Mar 26, 2020 •

edited

Loading

camallen Mar 26, 2020

chelseatroy Mar 26, 2020 •

edited

Loading

camallen Mar 26, 2020

camallen Mar 26, 2020

chelseatroy Mar 26, 2020

camallen Mar 26, 2020

chelseatroy Mar 26, 2020

camallen Mar 26, 2020

chelseatroy Mar 26, 2020

	<a href="{% url 'social:begin' 'panoptes' %}" class="theia-button">Login with Panoptes</a><br>
	<a href="{% url 'social:begin' 'zooniverse' %}" class="theia-button">Login with Zooniverse</a><br>


		def get_authenticated_panoptes(bearer_token, refresh_token, bearer_expiry):
		authenticated_panoptes = Panoptes(

	authenticated_panoptes = Panoptes(
	user_authenticated_panoptes = Panoptes(

	authenticated_client = Panoptes(
	server_authenticated_client = Panoptes(

Reimplement Authentication #94

Reimplement Authentication #94

Conversation

chelseatroy commented Mar 25, 2020

camallen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chelseatroy Mar 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chelseatroy Mar 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chelseatroy Mar 26, 2020 •

edited

Loading

chelseatroy Mar 26, 2020 •

edited

Loading