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

Change authentication method with OAuth #237

Open
wants to merge 2 commits into
base: testlink_1_9
Choose a base branch
from

Conversation

kenchan0130
Copy link
Contributor

@kenchan0130 kenchan0130 commented Jan 6, 2020

What I did

When using OAuth, I changed OAuth login specification like testlink users are linked by login column data instead of email column.

a12758f are main changes.

153baeb are format.

Motivation

Currently, when using OAuth, users are associated using the data in the email column of the users table.
This has several disadvantages.

  1. End users can freely change their email address.
    • End users can impersonate another user at their will.
  2. The email address has no unique guarantee.
    • User must be authenticated with a unique value.

@squash-labs
Copy link

squash-labs bot commented Jan 6, 2020

Manage this branch in Squash

Test this branch here: https://kenchan0130fix-oauth-login-dhaci.squash.io

@kenchan0130
Copy link
Contributor Author

kenchan0130 commented Apr 22, 2020

@ maintainer
How is this going?

@fmancardi
Copy link
Contributor

I need to understand if I want to add this to stable code or made it a configurable option

@kenchan0130
Copy link
Contributor Author

I don't think it's appropriate to associate users in OAuth with values that the end user is free to change.

That's why I recommend introducing this as a stable code.

@fmancardi
Copy link
Contributor

I understand but disagree with the idea of setting this as the standard way for oauth.
Surely a needed change is that email need to be populated and unique

@kenchan0130
Copy link
Contributor Author

I agree with you. In that case, the email should be given a UNIQUE contract.

In addition, if you are using an external provider, it would be better if the email address is only available to the administrator role user to change each user's email address, for example.

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

Successfully merging this pull request may close these issues.

2 participants