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.

Sign up for GitHub

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

Jump to bottom

[SSO] - [PR 4] - SSO/Token login functionality #453

Merged
merged 81 commits into from Aug 23, 2021
Merged

[SSO] - [PR 4] - SSO/Token login functionality #453

merged 81 commits into from Aug 23, 2021

Conversation

MidhunSureshR
Copy link
Contributor

No description provided.

bwindels
bwindels reviewed Aug 16, 2021
View reviewed changes
Copy link
Contributor

@bwindels bwindels left a comment

Choose a reason for hiding this comment

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

This looks good, and works well from a first try. Did my first review, didn't get all the way through yet though.

src/domain/login/SSOLoginViewModel.js Outdated Show resolved Hide resolved
src/domain/login/SSOLoginViewModel.js Outdated Show resolved Hide resolved
src/domain/login/SSOLoginViewModel.js Outdated Show resolved Hide resolved
src/domain/login/SSOLoginViewModel.js Outdated Show resolved Hide resolved
src/matrix/SessionContainer.js Outdated Show resolved Hide resolved
src/domain/navigation/index.js Outdated Show resolved Hide resolved
src/domain/login/SSOLoginViewModel.js Outdated Show resolved Hide resolved
src/domain/login/LoginViewModel.js Outdated Show resolved Hide resolved
src/domain/RootViewModel.js Outdated Show resolved Hide resolved
src/domain/login/LoginViewModel.js Outdated Show resolved Hide resolved
@MidhunSureshR
Add method to query login types
e0fcec9 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Create LoginMethod for password login
20765d9 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Rewrite password login to use PasswordLoginMethod
a53e297 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Make lint happy
acfe3f3 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Add license headers
72fb7f6 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Add explaining comment
46b7d9a 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Move files to separate directory
730a6b2 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Formatting fix
9651817 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Pass log as argument
f3946fc 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Allow sso to be a root segment
18e1c30 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Parse loginToken from query parameter
19664e5 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Add test
e2d2291 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Add method that returns callback url
3efadcb 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Add method to redirect to a specific URL
987a83b 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Ignore sso segment in url
74f5e30 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Support loginToken query in History
bed0185 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Add functionality to remove loginToken from URL
b261374 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Show link for SSO login
75d7171 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Show completion view on sso segment
4b87887 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Add view
2103adf 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Remove queryLoginFromInput()
2c953e3 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Move normalizeHomeserver to LoginViewModel
0af27fc 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Remove unnecessary braces
ce5fdd4 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Replace link with button
c82af5a 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Move view to correct directory
cabffd5 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Split login view into password and sso components
b8f0361 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Style sso button
93720f6 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Style CompleteSSOLoginView
6c6c4c7 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Save homeserver before redirecting
e424293 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Fix emit for isBusy
784b06d 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Set busy state from login vm
ff8417d 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Internationalize + add back old message
355468b 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Separate errors for each vm
a2677a6 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Red + bold error
55da588 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
No need to pass showError
82067ca 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Add spinner
a5985cb 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
No need to observe
0630452 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Add explaining comment
94ba93a 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Use homeserver from login method
0e6139d 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
add missing emit
d47e126 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Convert link to button
04806a1 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
bwindels
bwindels reviewed Aug 23, 2021
View reviewed changes
Copy link
Contributor

@bwindels bwindels left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, looking great! Just a few final nits, and we're there. Have tried it an it works great 👍

src/domain/login/CompleteSSOLoginViewModel.js Outdated Show resolved Hide resolved
src/matrix/SessionContainer.js Outdated Show resolved Hide resolved
src/domain/login/LoginViewModel.js Outdated Show resolved Hide resolved
src/domain/login/CompleteSSOLoginViewModel.js Outdated
_showError(message) {
this._errorMessage = message;
this.emitChange("errorMessage");
this._errorMessage = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why you clear it again after emitting? If any other emit in the viewmodel triggers a binding update, the error would just disappear at an unrelated timed. I'd expect to clear it only when you try to login again ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that wasn't very smart of me 🙂

Fixed for LoginViewModel and PasswordLoginViewModel but for CompleteSSOLoginViewModel I've removed this._errorMessage = "" since there's no UI currently for retrying a failed login using loginToken.

src/platform/web/ui/css/themes/element/theme.css Outdated Show resolved Hide resolved
src/domain/login/LoginViewModel.js Outdated Show resolved Hide resolved
@MidhunSureshR
Split logintoken into two words
84fd286 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Also check LoadStatus.Error
c9fbafb 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Do not set error message to empty string
ecfdc31 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Fix toggleBusy
69478b8 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Remove duplicate style
e80667c 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
More simpler navigation
b0db7e0 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Rename toggle to set
fc169af 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Catch any error from queryLogin
c9319c7 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
@MidhunSureshR
Make navigation simpler
ef4db4a 
Signed-off-by: RMidhunSuresh <rmidhunsuresh@gmail.com>
bwindels
bwindels approved these changes Aug 23, 2021
View reviewed changes
Copy link
Contributor

@bwindels bwindels left a comment

Choose a reason for hiding this comment

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

This is good to go!

@bwindels bwindels merged commit 3b693c5 into element-hq:master Aug 23, 2021
This was referenced Aug 25, 2021
Closed
Closed
Closed
@tommie tommie mentioned this pull request Sep 30, 2022
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwindels bwindels bwindels approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@MidhunSureshR @bwindels