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

Double entityid attributes #135

Closed
lairdbarrett opened this issue Mar 6, 2021 · 7 comments
Closed

Double entityid attributes #135

lairdbarrett opened this issue Mar 6, 2021 · 7 comments

Comments

@lairdbarrett
Copy link

Currently there are two entityid attributes used: entity_id and and entityID. This is to address a backwards-compatibility issue with older versions of the software.

Having two attributes can be a source of confusion for integrators that can lead to an inconsistency of user experience across integrators' sites.

The entityID attribute should not be needed.

Example below:

Integrator 1:

Dear...

We [an integrator] are very close to implementing the standard integration of SeamlessAccess in the coming days. We have performed a soft launch on our content platform and are about to embark
on UAT with a handful of customers across the globe.

Using login credentials provided to me by one of our UK subscribing libraries, I was able to test persistence on our website as well as across the various participating publisher sites. I was able to prove persistence works when a user makes known their chosen IdP on each participating publisher site, whether that’s ScienceDirect, or WileyOnline and then lands on a resource on our website. In all instances the SeamlessAccess button recognised the already confirmed IdP.

However, we noticed an issue with your website, whereby although the button changed to recognised the chosen IdP, when selecting the button to authenticate through that IdP, I was unable to. I noticed the same outcome having tested this between your website and other participating publisher sites.

We believe that you are using an alternative way in how they construct the SeamlessAccess object in the local browser storage. It looks like the entityID attribute is omitted, in that it only contains ‘entity_id’. So when a user successfully goes through the WAYF process on your website with their chosen IdP, and then bounces to our website, our site recognises the IdP on the button, so the persistence takes effect. However, when clicking on the SeamlessAccess button to authenticate as a user from that IdP, it does not point to the IdP SSO login page – in other words nothing happens and the user hits a barrier.

Having gone through the document below, my understanding is that both the ‘entityID ‘and ‘entity_id’ attributes should be contained in the SeamlessAccess object

Our concern here is this could now impede users who may land on an our website resource from your website. The user would be forced to use the ‘Add or Change institution’ link to then reconfirm themselves as a user from the very IdP they are being recognised as, introducing friction in a process designed to remove just that.

Please let me know your thoughts on this matter and I look forward to your reply.

Integrator 2:

Hi...

Thanks for raising this.

I think if I understand correctly, you’re using the entity_id attribute to populate the institution to the SA button, but the entityID attribute to construct the WAYFless URL that sits behind the button, and our website is only populating the entity_id attribute, so on your website the institution displays in the SA button, but there’s no WAYFless URL behind it.

It’s been a while since our implementation, but I think we only populated entity_id, because that was the forward-facing option. And I’m not sure that everyone using the SA service is using the entity_id attribute to populate the institution to the SA button and the entityID attribute to construct the WAYFless URL.

Seamless Access team: can I ask please?: Can you remind me why there are two entityid attributes? Should both be populated and why? And what should someone use to pull back the entityid to populate the name in the button and construct the WAYFless URL.

Thanks.

Seamless Access team:

The "double entityID attributes" is to address a backwards-compatibility issue with older
versions of the software. The entityID attribute should not be needed.

Could somebody open an issue on github.com/TheIdentitySelector/thiss-js and we'll get to
work debugging this.

@leifj
Copy link
Contributor

leifj commented Mar 8, 2021

Confirmed. I'm pretty sure the issue is in cta/index.js where we only look at entityID instead of using that as a fallback.

@leifj
Copy link
Contributor

leifj commented Mar 9, 2021

I've pushed a PR - can you test integration with https://deploy-preview-137--thiss.netlify.app/ and tell me if you can see any issues?

@Kublicki
Copy link

Kublicki commented Mar 9, 2021

Hey Leif,

I can still see both attributes present in the storage (some identifiers removed):

{ "last_refresh": 1615304223944, "last_use": 1615304223944, "entity": { "title": "MY ENTITY", "descr": "MY ENTITY", "title_langs": { "en": "University of ..." }, "descr_langs": { "en": "University of ..." }, "auth": "saml", "entity_id": "urn:...", "entityID": "urn:...", "type": "idp", "hidden": "false", "scope": "MY SCOPE", "domain": "MY SCOPE", "name_tag": "GTY", "id": "{sha1}..." } }

I would expect to see only entity_id - is that correct?

@leifj
Copy link
Contributor

leifj commented Mar 9, 2021

No you should still see both attributes. My fix simply deals with both of them in the standard integration.

@Kublicki
Copy link

Does it mean, if a service provider sets up only entity_id attribute (using advanced implementation) and another service provider uses standard implementation persistence service should work both ways between those service providers?

@leifj
Copy link
Contributor

leifj commented Mar 11, 2021

yes that is the idea

@leifj
Copy link
Contributor

leifj commented Apr 20, 2021

OK I'm gona mark this as closed. It will get rolled out to testing with a bunch of other stuff.

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

No branches or pull requests

3 participants