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

[NEW] Apple Login #24060

Merged
merged 9 commits into from
Jan 14, 2022
Merged

[NEW] Apple Login #24060

merged 9 commits into from
Jan 14, 2022

Conversation

ggazzo
Copy link
Member

@ggazzo ggazzo commented Dec 29, 2021

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

@ggazzo ggazzo force-pushed the feat/apple-oauth branch 3 times, most recently from 19f1fbb to 93e2f4a Compare December 29, 2021 22:38
@jeanfbrito
Copy link
Contributor

giphy

jeanfbrito
jeanfbrito previously approved these changes Jan 12, 2022
showButton: false,
secret,
enabled: settings.get('Accounts_OAuth_Apple'),
loginStyle: 'redirect',
Copy link
Member

Choose a reason for hiding this comment

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

should this just use config.loginStyle instead of having duplicated here?

scope: 'name email',
mergeUsers: true,
accessTokenParam: 'access_token',
loginStyle: 'redirect',
Copy link
Member

Choose a reason for hiding this comment

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

should we use 'popup' instead? I remember we discussing how 'redirect' could be bad for the desktop app.

Copy link
Member Author

Choose a reason for hiding this comment

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

I dont think this has any effect since we trigger the action by the apple script
but sure its a little messed

export const AppleOauthButton: FC = () => {
const enabled = useSetting('Accounts_OAuth_Apple');
const absoluteUrl = useAbsoluteUrl();
const appleClientID = useSetting('Accounts_OAuth_Apple_id') || '[CLIENT_ID]';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const appleClientID = useSetting('Accounts_OAuth_Apple_id') || '[CLIENT_ID]';
const appleClientID = useSetting('Accounts_OAuth_Apple_id');

@ggazzo ggazzo merged commit 1441feb into develop Jan 14, 2022
@ggazzo ggazzo deleted the feat/apple-oauth branch January 14, 2022 18:12
gabriellsh added a commit that referenced this pull request Jan 17, 2022
…ove/setup-wizard

* 'develop' of github.com:RocketChat/Rocket.Chat: (176 commits)
  [IMPROVE] Admin page header buttons consistency (#24168)
  i18n: Language update from LingoHub 🤖 on 2022-01-17Z (#24193)
  [FIX] Integration section crashing opening in My Account (#24068)
  [IMPROVE] Rewrite roomNotFound to React Component (#24044)
  Regression: Enable custom emoji on admin custom status page (#24186)
  Chore: Update Meteor to 2.5.3 (#24075)
  [NEW] Apple Login (#24060)
  Chore: Update Apps-Engine to 1.29.2 (#24171)
  feat: enabling emoji on custom status (#24170)
  [FIX] App Framework Enable hanging indefinitely (#24158)
  [FIX] CSV Importer failing to import users (#24090)
  Fix Engagement Dashboard API requests (#24142)
  Language update from LingoHub 🤖 (#24127)
  Chore: Migrate useOutsideClick to fuselage-hooks (#24133)
  Revert "Use fibers to store context"
  Use fibers to store context
  Chore: Include REG_TOKEN in docker-compose (#24123)
  [FIX] Custom Emoji Image preview #24117
  [IMPROVE] Added a Reset Button in the Account Profile Page (#24078)
  Revert: "[IMPROVE] Throw 404 error in invalid endpoints" (#24118)
  ...
@sampaiodiego sampaiodiego mentioned this pull request Jan 29, 2022
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.

3 participants