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

Add support for EOSC IdPs out of the box #348

Closed
wants to merge 6 commits into from

Conversation

gregcorbett
Copy link
Member

Use "voPersonID" to consume the user's identifier, rather than ePUID - as that has been deprecated following the (AAI teams?) adoption of the AARC-G026 guidelines.

Depends on #346

- Based on the original check made in the ShibToken.
  - https://github.com/GOCDB/gocdb/blob/5.8.1/lib/Authentication/AuthTokens/ShibAuthToken.php#L174
- it was removed from the ShibToken:
  - to allow the case where shib authentication fails but
    another mechanism (X.509, OIDC) succeeds. As such,
    the check shouldn't be added back to the ShibToken.
  - because we thought we'd be at the point where we could enforce
    authentication in apache and not need to in GOCDB. We aren't
    at that point yet - and we may not want to go there, prefering
    to enforce authentication in both apache and GOCDB for safety.
- Use "voPersonID" to consume the user's identifier
  - The ePUID has been deprecated following the (AAI teams?)
    adoption of the AARC-G026 guidelines.
@gregcorbett gregcorbett added this to the August 2022 milestone May 19, 2022
@gregcorbett gregcorbett self-assigned this May 19, 2022
@gregcorbett
Copy link
Member Author

Note to Future Greg: some of the 62 codeclimate issues are low hanging fruit and should be fixed before marking as ready for review.

@gregcorbett
Copy link
Member Author

This entitlement used in this will also need to be changed to the new one provided by EOSC AAI.

- This is needed to determine a default ID string for a user
- MyConfig1.php should be the source of what AuthTokens/Types
  are valid.
@gregcorbett
Copy link
Member Author

This entitlement used in this will also need to be changed to the new one provided by EOSC AAI.

This is now done.

@gregcorbett
Copy link
Member Author

Adding a new IdP feels more cumbersome now we have identity linking (maybe it's always been somewhat cumbersome).

The new IdP has to be added in three place.

  1. MyConfig1.php to enable GOCDB to resolve the info the new IdP gives it into an ID String
  2. A mapping of AuthTokens to authentication realms hardcoded in the User Service.
  3. In the PI code in order to expose the new ID Strings to the outside world.

I think point 2 can be addressed with some refactoring of the AuthTokens, such that one AuthToken class corresponds to one IdP. Though this is probably best attempted once we have dropped shibboleth support and only support OIDC. One AuthToken can then correspond to one authentication realm and the getAuthTypes could loop through $myConfig1->getAuthTokenClassList(); to determine a list of possible auth types - similar to what it does now, but without the need for the hardcoded mapping in this file (pushing it into the tokens - which feels like a better place for it).

I think point 3 could be solved in a similar way, by referencing $myConfig1->getAuthTokenClassList(); and a mapping between AuthToken and API XML tag.

I'll copy this into a separate issue.

More relevant to this PR, I have removed the valdidateAuthType method from the User Service. it felt somewhat circular to have MyConfig1.php accept the token as valid, only to then (in a perfect world) query MyConfig1.php again to re check it's validity.

@gregcorbett
Copy link
Member Author

gregcorbett commented Jun 30, 2022

I'll copy this into a separate issue.

#360

@gregcorbett
Copy link
Member Author

Superseded by #430

@gregcorbett gregcorbett removed this from the 5.11.0 milestone Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant