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

Extend userprofile endpoint with all data on the current user from Jupyter Server #510

Conversation

markgrahamdawson
Copy link
Contributor

@markgrahamdawson markgrahamdawson commented Oct 5, 2023

Closes cylc/cylc-ui#1504

Extends the userprofile endpoint to provide all the fields on current_user.
This is required for cylc/cylc-ui#1505

Available data

Available data Single user mode

In single user mode the user is authenticated with a bearer token. Inside the UserProfileHandler a function is_token_authenticated will return True.
In this case jupyter server knows no info about the user, so handler.current_user is populated with values that are not helpful:

display_name = "Anonoymous Herse"
username = "ae5071....."
name = Anonoymous Herse"
initials = "AH"

Available data Multi user / hub mode

In multiuser (hub) mode the user provides a username and password to the server for auth. Inside the UserProfileHandler a functionis_token_authenticated will return False. In this case the jupyter server knows the users username so uses that for populating handler.current_user.

display_name = "mdawson"
username = "mdawson"
name = "mdawson"
initials = none

Note: initials set to none

This is what we have to work with to try and send some useful data back to the /user-profile/ endpoint.

The approach

The username used for hub login depends on how jupyterhub is configured and can potentially accept more than one value. For example jsmith and [email protected] may both be valid.

An alternative method for attaining a name for the user is to get it from the operating system using getpass (ME = getpass.getuser()) .

Approach for Multi user / hub mode

Use the username that can be accessed through jupyter on handler.current_user.
Calculate a value for the initials field.

Approach for Single user

Get a username from the OS
Calculate a value for the initials field.

Example responses

If authenticated and have a . in username

username	"m.dawson"
name	"m.dawson"
display_name	"Anonymous Herse"
initials	"MD"

If authenticated and dont have a . in username

username	"mdawson"
name	"mdawson"
display_name	"Anonymous Herse"
initials	"M"

https://jupyter-server.readthedocs.io/en/latest/operators/security.html#jupyter_server.auth.User

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • Tests are included (or explain why tests are not needed). - See comment
  • CHANGES.md entry included if this is a change that can affect users - Not added but can do if required?
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@markgrahamdawson markgrahamdawson self-assigned this Oct 5, 2023
@markgrahamdawson markgrahamdawson changed the title added all fields on current_user to endpoint Extend userprofile endpoint with all data on the current user from Jupyter Server Oct 5, 2023
@markgrahamdawson
Copy link
Contributor Author

Tests for this part of the code are in test_handlers.py and test_auth.py. There is a test to check whats returned from this endpoint contains the 'name' field so Im not sure it needs anything more than that

@markgrahamdawson markgrahamdawson marked this pull request as ready for review October 5, 2023 14:12
@oliver-sanders oliver-sanders added this to the 1.5.0 milestone Oct 5, 2023
@markgrahamdawson
Copy link
Contributor Author

After discussion with @oliver-sanders :
If token is authenticated we will need to figure out a means of getting the initials on the server-side and set username and name to be the same.

--suggest--
if '.' in name get first letter and first letter after '.'
otherwise just take first letter

@oliver-sanders
Copy link
Member

Sadly, when token authenticated, Jupyter Server appears to set:

username: <token>
name: <username>
display name: <random>
initials: <random>

Which isn't helpful for what we're trying to achieve.

In Cylc we make the assumption that the bearer of the token is the user the server is running as. This isn't technically true, you could give another user your token, however, this is how it's intended to work.

Mark Dawson added 2 commits October 10, 2023 15:30
If not authenticated:

username	"ae507119171a4306bb631b4b01d5ed82"
name	"ae507119171a4306bb631b4b01d5ed82"
display_name	"Anonymous Herse"
initials	"AH"

If authenticated and have a . in username

username	"m.dawson"
name	"m.dawson"
display_name	"Anonymous Herse"
initials	"MD"

If authenticated and dont have a . in username

username	"mdawson"
name	"mdawson"
display_name	"Anonymous Herse"
initials	"M"
@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (1433411) 75.42% compared to head (1a79546) 74.80%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #510      +/-   ##
==========================================
- Coverage   75.42%   74.80%   -0.62%     
==========================================
  Files          12       12              
  Lines        1428     1437       +9     
  Branches      236      239       +3     
==========================================
- Hits         1077     1075       -2     
- Misses        301      309       +8     
- Partials       50       53       +3     
Files Coverage Δ
cylc/uiserver/handlers.py 75.31% <28.57%> (-4.55%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Return empty string for initials if user is not authenticated
then on the front end if there is no value for initials we can show a
user edit icon.

Also changed the way in whic initials were calculated if there is a '.'
to ensure only first character of the first name is taken
@MetRonnie
Copy link
Member

MetRonnie commented Oct 12, 2023

If not authenticated:

username	"ae507119171a4306bb631b4b01d5ed82"
name	"ae507119171a4306bb631b4b01d5ed82"
display_name	"Anonymous Herse"
initials	""

Surely this can't be "not authenticated", as someone who is not authenticated (either by a token or logging on in Jupyter Hub) cannot access the app? Are you talking about the token authenticated case which seems similar to Oliver's comment:

when token authenticated, Jupyter Server appears to set:

username: <token>
name: <username>
display name: <random>
initials: <random>

@MetRonnie
Copy link
Member

Or is it for the case that a site has turned off authentication in the Cylc UIServer config?

@markgrahamdawson
Copy link
Contributor Author

as someone who is not authentic

https://github.com/cylc/cylc-uiserver/pull/510/files#diff-2091a8b80ef334090a743e363864144410629a3f4695014f9b1df91d9231c6e3L131-L136

This is the related change in the uiserver code

@MetRonnie
Copy link
Member

That is someone who is not token authenticated, so presumably password authenticated?

@oliver-sanders
Copy link
Member

The user has to be authenticated (the @web.authenticated on the UserProfileHandler ensures this), the difference is between token authentication (which includes the built-in password authentication) where the server is accessed via a bearer token which doesn't tell us anything about the bearer (i.e. authenticated user); and other authentication methods which may be able to provide information about the authenticated user.

@MetRonnie
Copy link
Member

MetRonnie commented Oct 17, 2023

So then in what circumstance do you get this?

username	"ae507119171a4306bb631b4b01d5ed82"
name	"ae507119171a4306bb631b4b01d5ed82"
display_name	"Anonymous Herse"
initials	""

Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. Got a couple of suggestions to tidy the code a little

cylc/uiserver/handlers.py Outdated Show resolved Hide resolved
cylc/uiserver/handlers.py Outdated Show resolved Hide resolved
@oliver-sanders oliver-sanders merged commit 9b23154 into cylc:master Oct 25, 2023
3 checks passed
MetRonnie pushed a commit to MetRonnie/cylc-uiserver that referenced this pull request Nov 2, 2023
…pyter Server (cylc#510)

Extend userprofile endpoint with all data on the current user from Jupyter Server
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Source user avatar initials from Jupyter Server
4 participants