Skip to content
This repository has been archived by the owner on Feb 8, 2025. It is now read-only.

feat: add external login providers #1174

Merged
merged 19 commits into from
Jul 12, 2021
Merged

feat: add external login providers #1174

merged 19 commits into from
Jul 12, 2021

Conversation

asbiin
Copy link
Contributor

@asbiin asbiin commented Jul 9, 2021

This is a big PR allowing external providers login.

image

You fool, don't forget these steps:

  • Unit tests
  • Tests with Cypress
  • Documentation
  • Dummy data

@asbiin asbiin requested a review from a team as a code owner July 9, 2021 19:59
@asbiin asbiin requested review from djaiss and removed request for a team July 9, 2021 19:59
@asbiin asbiin marked this pull request as draft July 9, 2021 20:00
Copy link
Member

@djaiss djaiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I still need to run this PR locally to check the UI but from a code stand point, it's great!

@asbiin
Copy link
Contributor Author

asbiin commented Jul 10, 2021

I still need to add some tests ...

@asbiin
Copy link
Contributor Author

asbiin commented Jul 10, 2021

Some documentation would also help!!!

@djaiss
Copy link
Member

djaiss commented Jul 10, 2021

@asbiin This is what I have:

image

What did I miss?

@asbiin
Copy link
Contributor Author

asbiin commented Jul 10, 2021

@djaiss you have to activate the providers by setting: LOGIN_PROVIDERS=azure,github,google,linkedin,saml2,slack,twitter (the order is the order in the UI)

@djaiss
Copy link
Member

djaiss commented Jul 10, 2021

@asbiin thanks.

Then we should add something in this PR: if the LOGIN_PROVIDERS is not set, disable the Login with part in the view.

@djaiss
Copy link
Member

djaiss commented Jul 10, 2021

By default this feature should not be active, I'd say.

@djaiss
Copy link
Member

djaiss commented Jul 10, 2021

Also, shouldn't this be on the Signup form as well?

@asbiin
Copy link
Contributor Author

asbiin commented Jul 10, 2021

Then we should add something in this PR: if the LOGIN_PROVIDERS is not set, disable the Login with part in the view.

By default this feature should not be active, I'd say.

It's fixed, it was not intended.

@asbiin
Copy link
Contributor Author

asbiin commented Jul 10, 2021

Also, shouldn't this be on the Signup form as well?

I'm not sure, the register page is for login/password form, not sure if it's useful to duplicate it there.

@asbiin
Copy link
Contributor Author

asbiin commented Jul 10, 2021

Also todo in other PRs:

  • Update profile view to see providers: which one are connected, and option to connect other providers.
  • Add a screen between auth and user creation so the user can check the email address and other information.

@asbiin asbiin marked this pull request as ready for review July 12, 2021 17:48
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

78.6% 78.6% Coverage
0.0% 0.0% Duplication

@asbiin asbiin merged commit e92d516 into main Jul 12, 2021
@asbiin asbiin deleted the 20210707-socialite branch July 12, 2021 18:27
@github-actions
Copy link
Contributor

🎉 This PR is included in version 0.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mannp
Copy link

mannp commented Jul 30, 2021

This would be cool as a backport to monica 👍🏻

@github-actions
Copy link
Contributor

This pull request has been automatically locked since there
has not been any recent activity after it was closed.
Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants