search

pombase / canto

The PomBase community curation tool
https://curation.pombase.org
Other
19 stars 7 forks source link

Use ORCID API / OAuth2 for logins #1197

Open kimrutherford opened 8 years ago

kimrutherford commented 8 years ago

We should outsource the confirming of username and password ORCID. Only admins need to login at the moment so requiring ORCIDs shouldn't be a problem.

Documentation starts here: http://orcid.org/organizations/integrators

kimrutherford commented 8 years ago

It's been a fight but I have the basics of this working now. Tomorrow I'll tidy things up, add better error checking and deploy it in the test Canto for you to try.

kimrutherford commented 8 years ago

The ORCID admin login code is now in the test Canto. I think it's more or less OK. If you try it at the moment you'll get "no such user". You'll need to let me know your ORCID so I can add it to the Canto database.

Antonialock commented 8 years ago

I'm sure i added my Orchid id to the last session i made (live tool). Was i supposed to be able to do that?

On Friday, 7 October 2016, Kim Rutherford notifications@github.com wrote:

The ORCID admin login code is now in the test Canto. I think it's more or less OK. If you try it at the moment you'll get "no such user". You'll need to let me know your ORCID so I can add it to the Canto database.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/pombase/canto/issues/1197#issuecomment-252126566, or mute the thread https://github.com/notifications/unsubscribe-auth/AMI00ilA7e5YpINV78AZG_A67eN6RXgwks5qxZVAgaJpZM4JUDM7 .

Antonia Lock, PhD PomBase Biocurator, http://www.pombase.org Department of Genetics, Evolution and Environment, The Darwin Building, University College London London WC1E 6BT, UK

kimrutherford commented 8 years ago

I'm sure i added my Orchid id to the last session i made (live tool).

Thanks, looks like you did. I've copied your ORCID to the test Canto so you should be able to log in there.

Was i supposed to be able to do that?

Yep! But that input field for ORCID is kind of temporary. For now we're just collecting the ORCIDs.

The next plan is to replace the orcid field with a button that takes the user to the ORCID web site to confirm their ID.

kimrutherford commented 8 years ago

If fixed some bugs with the ORCID login. Let me know your ORCID if you want to give it try in the test Canto.

ValWood commented 8 years ago

I added my ORCID via a new session. Did it work?

Note that when I first start to fill in my details, it prefills my e-mail in the ORCID field, is there a way to prevent this?

orcid

ValWood commented 8 years ago

And now on a another session, I am prompted again for my ORCID with a blank field...

kimrutherford commented 8 years ago

I added my ORCID via a new session. Did it work?

It didn't - I'll look into that.

it prefills my e-mail in the ORCID field, is there a way to prevent this?

I think so. Which browser are you using?

ValWood commented 8 years ago

chrome

kimrutherford commented 8 years ago

And now on a another session, I am prompted again for my ORCID with a blank field...

I've fixed that now. Could you try again?

kimrutherford commented 8 years ago

it prefills my e-mail in the ORCID field, is there a way to prevent this?

I can't reproduce that at the moment - my Chrome doesn't prefill any fields. I'll work on it.

ValWood commented 8 years ago

I still see this when I begin to type my name:

autofil

I added my ORCID...

orcid

I then went to claim a new session, I'm still prompted for my ORID and it still prefills my name:

promopted

Also tried via "session reassignment" and see the same: (my details are there, but ORCID field empty:

session reassignment orcid empty

kimrutherford commented 7 years ago

I added my ORCID...

I spotted one problem, you had a "orcid.org/" prefix but the code was expecting just the ID like: "0000-0001-6277-726X". The code also just ignores IDs that don't look right which isn't great for the user.

I've fixed it so that the "orcid.org/" prefix is allowed (but optional). Could you try a session in the test Canto again some time?

I plan fix it so that it displays a nice message if the ORCID format is wrong.

I then went to claim a new session, I'm still prompted for my ORID and it still prefills my name:

I haven't worked out that one yet.

kimrutherford commented 6 years ago

I need to get back to this.

The branch is here: https://github.com/pombase/canto/commits/tickets/ticket-1197

kimrutherford commented 1 year ago

I've spent today updating the prototype code for logging in as admin using ORCID. The basic functionality seems to work, with a few rough edges. I'll get this working and deployed then move on to #1409 and #1269.

The current implementation is on a separate branch for now.

kimrutherford commented 1 year ago

It took me it little while but the new ORCID login system is set on https://curation.pombase.org/test/

kimrutherford commented 1 year ago

It took me it little while but the new ORCID login system is set on https://curation.pombase.org/test/

Hi @ValWood and @manulera

When you get a chance could you try logging in to the test Canto as admin using the menu? You should be redirected to ORCID where you'll need to do user name and password (unless you logged in recently). ORCID will redirect you back to Canto where you should be logged in. If that doesn't work please let me know.

If that does work could you then log out (using the menu) then go to https://curation.pombase.org/test/view/list/triage_phenotype_screen_publications?model=track (which is just an example of a non-top-level page) and try the log in link?:

image

Thanks very much!

Even if that all works, there is still a little bit of fixing to do before deploying to the main site. I should be able to do that by Monday.

manulera commented 1 year ago

Worked for me on both. Actually I had seen it yesterday in the canto test server when I went to check whether the comments in curation terms were shown 👀

kimrutherford commented 1 year ago

Worked for me on both.

Thanks for checking!

ValWood commented 1 year ago

worked for me too.

kimrutherford commented 1 year ago

worked for me too.

Thanks. I'll update the main Canto in the next day or two.

I have a few questions about ORCID and APICURON for the next Zoom call.

kimrutherford commented 1 year ago

Thanks for the testing. I'll update the main Canto over the weekend. It will need quick test on Monday morning.

I'd had a good look at how we currently collect names, emails and ORCIDs and I think it's fine. It's probably OK for ORCIDs to be optional but strongly encouraged. If the user doesn't want to give an ORCID (or can't be bothered) it's not the end of the world for us. The main thing for us is to get the annotation.

But like we talked about it makes sense to add a sentence or two on the benefits for the user and for PomBase of adding an ORCID. If users understand the reasoning they're more likely to bother.

For returning users, all the fields including the ORCID should be pre-filled in so they should only need to put it in once.

And as we discussed, a form to adding an ORCID that we can send to existing users will help us collect more.

image

kimrutherford commented 1 year ago

I'll update the main Canto in the next day or two.

I forgot. I'll try to remember before Monday.

kimrutherford commented 1 year ago

I'll update the main Canto

I finally remembered. Please let me know if you have trouble logging in. If it's all still working OK we can probably close this issue (at last).

@jseager7 we should have a call about this at some point to talk about if this change will be OK for PHI-Canto. There is a bit of configuration involved.

jseager7 commented 1 year ago

@kimrutherford Was ORCID authentication required for integration with APICURON? If so, that's one of our long-term aims for PHI-base too, so this should be useful for us.

kimrutherford commented 1 year ago

Hi @jseager7

ORCID authentication doesn't help much with APICURON. We thought it would but we're reconsidering now.

We are looking at ways to improve how we handle curator (rather than admin) ORCIDs but we now think it's different to the problem of admin login using ORCID authentication.

jseager7 commented 1 year ago

@kimrutherford I actually set this up myself on our PHI-Canto server yesterday (having not updated the source code for ages).

I didn't realise at the time I wrote the above comments that the old authentication method would stop working when OAuth was enabled. As a result, we locked ourselves out of the admin interface and I had to scramble to get the correct configuration in place :sweat:

Fortunately I was able to follow the instructions on the ORCID website and your previous commits to get the authentication working.

The example given in the Canto documentation for Configuring ORCID for logins (shown below) is outdated: it uses a different configuration object and doesn't include a bunch of required properties like grant_uri, token_uri, authenticator, and scope. Notably, the value for the scope property isn't even defined in canto.yaml (I assumed it should be /authenticate based on the message for commit 731e32d).

authentication:
  orcid:
    client_id: ...
    client_secret: ...

Compare this to the template in canto.yaml:

oauth:
  authenticator: ORCID
  grant_uri: 'https://orcid.org/oauth/authorize'
  token_uri: 'https://orcid.org/oauth/token'
  client_id: 'CONFIGURE_IN_CANTO_DEPLOY.YAML'
  client_secret: 'CONFIGURE_IN_CANTO_DEPLOY.YAML'
  scope: 'CONFIGURE_IN_CANTO_DEPLOY.YAML'

(I'm assuming all the missing properties have to be specified, since IIRC Canto won't merge configuration objects between canto.yaml and canto_deploy.yaml.)

For reference, here's our configuration for OAuth (sensitive information replaced with placeholders).

oauth:
  authenticator: ORCID
  grant_uri: 'https://orcid.org/oauth/authorize'
  token_uri: 'https://orcid.org/oauth/token'
  client_id: 'ORCID_CLIENT_ID'
  client_secret: 'ORCID_CLIENT_SECRET'
  scope: '/authenticate'
kimrutherford commented 1 year ago

The example given in the Canto documentation for Configuring ORCID for logins (shown below) is outdated: it uses a different configuration object and doesn't include a bunch of required properties like grant_uri, token_uri, authenticator, and scope.

Sorry about that! It's good to hear that you got it work.

I'll fix the documentation before closing this issue.

jseager7 commented 1 year ago

I'll fix the documentation before closing this issue.

@kimrutherford Thanks.

By the way, how do you use the OAuth authentication system on test versions of Canto? When testing this locally I used the ORCID sandbox testing server and followed the instructions for authenticating ORCID IDs on the sandbox server (see here).

I was just wondering whether there's any way to disable the authentication or to use some fake system, just because it presumably won't be possible to test the admin interface without an internet connection now.

kimrutherford commented 1 year ago

Yep, that's a good point. Testing requires a internet connection and to use the sandbox, as you did.

I was just wondering whether there's any way to disable the authentication or to use some fake system

Unfortunately there isn't at the moment. But we will have to add something because the Canto test server currently doesn't allow logins. Previously we allowed admin access without logging in so users could see how admin worked without an account.