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

[NEW] Custom OAuth #1093

Merged
merged 5 commits into from
Aug 7, 2019
Merged

[NEW] Custom OAuth #1093

merged 5 commits into from
Aug 7, 2019

Conversation

jbuechele
Copy link
Contributor

@RocketChat/ReactNative

Closes #1018

@CLAassistant
Copy link

CLAassistant commented Jul 31, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@jbuechele jbuechele changed the title added oauth custom support [Feature] added oauth custom support Jul 31, 2019
@diegolmello
Copy link
Member

@jbuechele Thanks for your work!
Can you fix lint?
I'll review your PR right after.
Cheers.

@jbuechele
Copy link
Contributor Author

The lint errors were fixed.

@diegolmello diegolmello requested a review from philipbrito August 1, 2019 16:41
@philipbrito philipbrito changed the title [Feature] added oauth custom support [NEW] Custom OAuth Aug 2, 2019
philipbrito
philipbrito approved these changes Aug 4, 2019
View reviewed changes
Copy link
Contributor

@philipbrito philipbrito left a comment

Choose a reason for hiding this comment

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

Functionality is working fine. As further improvement we can render the custom OAuth button by getting its style from the backend.

Yours:

Screen Shot 2019-08-04 at 13 26 28

Ours:

Screen Shot 2019-08-04 at 13 33 46

Server:

Screen Shot 2019-08-04 at 13 36 34

Thank you!

@rasos
Copy link

rasos commented Aug 6, 2019

Very useful, looking forward to have this PR in the next release.

@jbuechele
Copy link
Contributor Author

OAuth

Should I change it or have you already changed it? But we could also just render the icon of oauth or openid connect? Then the buttons would look consistent.

@philipbrito philipbrito mentioned this pull request Aug 6, 2019
Merged
philipbrito
philipbrito suggested changes Aug 6, 2019
View reviewed changes
Copy link
Contributor

@philipbrito philipbrito left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Wait, we have a bug!

Try connection with a different server, then go to the server which supports custom OAuth authentication -> the login "method" list from the first server is being displayed on the second one.

@jbuechele
Copy link
Contributor Author

Wait, we have a bug!

Try connection with a different server, then go to the server which supports custom OAuth authentication -> the login "method" list from the first server is being displayed on the second one.

The bug is fixed. The Reducer has merged the old login services with the new ones

@diegolmello
Copy link
Member

@jbuechele Great job! I'll review/merge it today! :)

diegolmello
diegolmello approved these changes Aug 7, 2019
View reviewed changes
Copy link
Member

@diegolmello diegolmello left a comment

Choose a reason for hiding this comment

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

LGTM

@diegolmello diegolmello merged commit 6a3523d into RocketChat:develop Aug 7, 2019
@diegolmello
Copy link
Member

Merged. Thanks, @jbuechele! 👊

@yeoldegrove
Copy link

@diegolmello Will you bump the version, so that I can download this from google play?

@diegolmello
Copy link
Member

@yeoldegrove Yes, I'm fixing a few bugs on RN update and I'll release this soon.
First I have to review and merge #1108 and #1116 also.

djorkaeffalexandre pushed a commit that referenced this pull request Aug 9, 2019
@jbuechele @diegolmello
[NEW] Custom OAuth (#1093)
8169981
@lorek123 lorek123 mentioned this pull request Sep 19, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philipbrito philipbrito philipbrito requested changes

@diegolmello diegolmello diegolmello approved these changes

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

Successfully merging this pull request may close these issues.

Support Custom OAuth
6 participants
@jbuechele @CLAassistant @diegolmello @rasos @yeoldegrove @philipbrito