Sso - Githubissues

FORTH-ICS-INSPIRE / artemis-web

The web frontend of the ARTEMIS software project (https://github.com/FORTH-ICS-INSPIRE/artemis).

BSD 3-Clause "New" or "Revised" License

5 stars 8 forks source link

Sso #152

Closed CuriouzK0d3r closed 2 years ago

CuriouzK0d3r commented 2 years ago

Description of PR

Added Google SSO

What component(s) does this PR affect?

[ ] Back-End (Node.js, Next.js)
[x] Front-End (React.js)
[ ] Theming (CSS, React theme)
[ ] Build System
[ ] Docs

Does the PR require changes on other components? If yes, please mark the components:
[ ] Back-End (Node.js, Next.js)
[ ] Front-End (React.js)
[ ] Theming (CSS, React theme)
[ ] Build System
[ ] Docs

Related Issue

Resolves #

Solution

Type

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
[ ] Docs update
[ ] None of the above

Checklist:

[ ] I have read the contributing guide and my code conforms to the guidelines.
[ ] This change requires a change in the documentation.
[ ] I have updated the documentation accordingly.

)`>>>)>

gitpod-io[bot] commented 2 years ago

CuriouzK0d3r commented 2 years ago

LGTM at first sight, could you also write some documentation on using the new feature (+ screenshots if you have)? Also how can the admin limit the users that enter the system, are the google-logged first-time users entering the "pending approval" workflow as discussed? Otherwise new google logged-in users can become users without any admin approval.

A new user using the Google SSO will be added with 'pending' privileges. So it is like creating a new user with credentials.

CuriouzK0d3r commented 2 years ago

Screenshot from 2022-04-11 17-00-46

vkotronis commented 2 years ago

Maybe instead of "Login with Google" we should have a "Login via SSO" button which, when pressed, shows the available SSO login options in drop-down? We can start with a single one (Google), it seems more scalable to treat it this way I think (plus we may add more SSO options in the drop-down menu in the future)

CuriouzK0d3r commented 2 years ago

Screenshot from 2022-04-11 20-26-38

What do you think?

vkotronis commented 2 years ago

What do you think?

Yes that is fine

vkotronis commented 2 years ago

As discussed, we need also documentation on this:

Build Google app and get env vars
New user login
Existing user login
- screenshots Please add in-PR

vkotronis commented 2 years ago

@CuriouzK0d3r could you check if the bug with the mongo-stored user is resolved and update the PR with the master base? Then it can be merged I think, LGTM