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

IdP selector fails authentication when there are accounts with same usernames #17

Closed
LucaCinquini opened this issue Feb 2, 2016 · 5 comments
Assignees
Labels
Milestone

Comments

@LucaCinquini
Copy link
Member

Currently the IdP selector uses UserInfoDao (in security module) to look up accounts by username only. The authentication will fail if there's more than one account with the same username, for example:

esgcet=# select * from esgf_security.user;
id | firstname | middlename | lastname | email | username | password | dn | openid | organization | organization_type | city | state | country | status_code | verification_token
| notification_code
----+-----------+------------+----------+-----------------------+-----------+------------------------------------+----+----------------------------------------------------------+--------------+-------------------+------+-------+---------+-------------+-----------------------------------
---+-------------------
1 | Admin | | User | esgf-dev@jpl.nasa.gov | rootAdmin | $1$z0PA/.T.$gBTzp7Jfsx/8QM0r/I/D30 | | https://esgf-dev.jpl.nasa.gov/esgf-idp/openid/rootAdmin | Institution | | City | State | Country | 1 | c5ae43b6-5573-4f51-8d16-724e3c0c3f
2d | 0
2 | Admin | | User | esgf-dev@jpl.nasa.gov | rootAdmin | | | https://esgf-node.jpl.nasa.gov/esgf-idp/openid/rootAdmin | | | | | | 1 |
| 0
(2 rows)

@LucaCinquini LucaCinquini self-assigned this Feb 2, 2016
@LucaCinquini LucaCinquini added this to the Release 2.7.4 milestone Feb 2, 2016
@LucaCinquini
Copy link
Member Author

Fixed by using the value of "esgf.idp.peer" to sub-select the accounts that have the same username.

@philipkershaw
Copy link

Wouldn't it make sense to use OpenID uri to uniquely identifier users?

@LucaCinquini
Copy link
Member Author

Yes... but the IdP selector back-end was built to use the username as see (which is not unique).

@philipkershaw
Copy link

Sounds like you have a solution but there are some standard patterns I've seen for integrating SSO with internal identifiers. - One other option would be to assign a fresh uuid to the internal accounts the first time someone logs in from outside with an external OpenID. Don't want to stop you're fix now but perhaps we can talk about at a later date.

@LucaCinquini
Copy link
Member Author

Hi Phil,
indeed all of these suggestion are possible and good practices, but they would require some re-engineering of the database. We can schedule the work if needed... the fix I have applied is consistent with the previous implementation and seems to work, so I think it's ok to release it now. Thanks though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants