Skip to content

REFACTOR Remove extraneous user profile support from UrsIdp and the UserProfile class #252

@ndp-opendap

Description

@ndp-opendap

Remove the code in the OLFS that uses an "EDL user profile" in the authentication process. Once @hannahilea completes HYRAX-1875 I think we no longer need that code. This is worth doing because it greatly reduce the size of our session state entries and obviate the need for the UserProfile class to have all the state associated with managing that external user profile information.

Another thing that I think should be examined is to either make a child class of UserProfile, say EdlUserProfile to embody all of the EDL specific things, OR move all of the EDL specific operations out of UserProfile and into UrsIdp (which at this point could be renamed EdlIdp). This would all our UserProfile class to be simpler and more generic - The the user id and setUserFootPrint() and validateUserFootPrint() methods and their associated state would remain as these seem fundamental to the whole profile/session scheme.

Personally I prefer the child class EdlUserProfile pattern.

We could also I think move things from UrsIdp into UserProfile and/or EdlApi class:

  • UrsIdp.revokeEdlTokens(UserProfile userProfile) --> EdlUserProfile.revokeEdlTokens() -> EdlApi.revoke(token)
  • UrsIdp.invalidate(UserProfile userProfile) --> UserProfile.invalidate()

Also I noticed in UrsIpd, line 661- 674 :

        String uid = null;
        if (!getUrsClientAppPublicKeys().isEmpty()) {
            uid = getEdlUserIdFromToken(getUrsClientAppPublicKeys(), edlat.getAccessToken());
            if (uid == null) {
                log.error("{}Unable to get EDL user id from token locally; falling back to remote user id request", ERR_PREFIX);
            } else {
                userProfile.setUID(uid);
            }
        }
        if (uid == null) {
            userProfile.setUID(uid);
            getEDLUserProfile(userProfile);
        }

I think I would move the test:

        if (!getUrsClientAppPublicKeys().isEmpty()) 

Into the getEdlUserIdFromToken() method which already checks for a null access token and returns a null uid value;
Which would simplify things in the UrsIdp.doLogin() method:

    String uid = = getEdlUserIdFromToken(getUrsClientAppPublicKeys(), edlat.getAccessToken());
    if (uid == null) {
        log.error("{}Unable to get EDL user id from token locally; falling back to remote user id request", ERR_PREFIX);
        // userProfile.setUID(uid); //  Do we need to set this to null?
        getEDLUserProfile(userProfile);
    } else {
        userProfile.setUID(uid);
    }
    log.info("URS UID: {}", userProfile.getUID());  

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions