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

[FIX] Wordpress oAuth authentication wasn't behaving correctly #10550

Merged
merged 2 commits into from
Apr 24, 2018
Merged

[FIX] Wordpress oAuth authentication wasn't behaving correctly #10550

merged 2 commits into from
Apr 24, 2018

Conversation

kaiiiiiiiii
Copy link
Contributor

Closes #10492

wordpress_oauth

@cardoso
Copy link
Contributor

cardoso commented Apr 23, 2018

Nice @kaiiiiiiiii thanks!

I know it wasn’t in the issue but can you also change the scope to the minimum one? It’s asking for too many permissions.

Also, requesting a deploy here 🙂

@theorenck theorenck added this to the 0.65.0 milestone Apr 23, 2018
@theorenck theorenck added this to Desirable in May/2018 via automation Apr 23, 2018
@theorenck theorenck moved this from Desirable to Review/QA in May/2018 Apr 23, 2018
@kaiiiiiiiii
Copy link
Contributor Author

kaiiiiiiiii commented Apr 23, 2018

Done that. Let me know if there's anything else I can do :)

screen_shot_2018-04-23_at_20_46_46

@graywolf336
Copy link
Contributor

@RocketChat/core any reason we can't add this to the v64 release? Because it is a login issue and delaying it to the next release feels wrong to me.

@rodrigok rodrigok changed the title [FIX] Wordpress oAuth authentication [FIX] Wordpress oAuth authentication wasn't behaving correctly Apr 24, 2018
@rodrigok rodrigok added this to Desireable in 0.64.0 via automation Apr 24, 2018
@rodrigok rodrigok modified the milestones: 0.65.0, 0.64.0 Apr 24, 2018
@rodrigok rodrigok removed this from Review/QA in May/2018 Apr 24, 2018
@rodrigok rodrigok merged commit 159ad34 into RocketChat:develop Apr 24, 2018
0.64.0 automation moved this from Desireable to Done Apr 24, 2018
@kaiiiiiiiii kaiiiiiiiii deleted the fix_wordpress_oauth_authentication branch April 24, 2018 20:58
trongthanh added a commit to goalifyplus/Goalify.Chat that referenced this pull request Apr 27, 2018
* develop: (79 commits)
  fixed problems with margin negative (RocketChat#10558)
  Add some information regarding Zapier and Bots to the integrations page (RocketChat#10574)
  Added target="_blank" to homepage and support link. (RocketChat#10575)
  [FIX] Stop Firefox announcement overflowing viewport (RocketChat#10503)
  [FIX] Wordpress oAuth authentication wasn't behaving correctly (RocketChat#10550)
  Fix inconsistent response of settings.oauth endpoint (RocketChat#10553)
  Regression: Remove added mentions on quote/reply (RocketChat#10571)
  Fix the attachments and fields incorrectly failing on validation (RocketChat#10573)
  Deps update (RocketChat#10549)
  Map consumerKey to clientId (fix Twitter) (RocketChat#10560)
  Regression: Webhooks breaking duo to a too restrict test (RocketChat#10555)
  Fix issues with the rooms and apps (RocketChat#10559)
  Fix regression with announcement bar being displayed without content (RocketChat#10554)
  LingoHub based on develop (RocketChat#10545)
  Regression: Revert announcement structure (RocketChat#10544)
  Regression: Upload was not working (RocketChat#10543)
  Remove duplicated key from en.i18n.json
  Included missing lib (RocketChat#10532)
  dependencies update
  [NEW] Option to mute group mentions (@ALL and @here) (RocketChat#10502)
  ...

# Conflicts:
#	packages/rocketchat-i18n/i18n/en.i18n.json
#	packages/rocketchat-i18n/i18n/vi-VN.i18n.json
#	packages/rocketchat-ui-flextab/client/tabs/membersList.html
#	packages/rocketchat-ui-message/client/popup/messagePopupConfig.js
#	packages/rocketchat-ui-sidenav/client/sidebarHeader.js
nguyendichtu91295 added a commit to goalifyplus/Goalify.Chat that referenced this pull request Apr 27, 2018
…into goalify

* 'goalify' of https://github.com/goalifyplus/Goalify.Chat: (80 commits)
  Update gitlab, npm package lock, include current server update script
  fixed problems with margin negative (RocketChat#10558)
  Add some information regarding Zapier and Bots to the integrations page (RocketChat#10574)
  Added target="_blank" to homepage and support link. (RocketChat#10575)
  [FIX] Stop Firefox announcement overflowing viewport (RocketChat#10503)
  [FIX] Wordpress oAuth authentication wasn't behaving correctly (RocketChat#10550)
  Fix inconsistent response of settings.oauth endpoint (RocketChat#10553)
  Regression: Remove added mentions on quote/reply (RocketChat#10571)
  Fix the attachments and fields incorrectly failing on validation (RocketChat#10573)
  Deps update (RocketChat#10549)
  Map consumerKey to clientId (fix Twitter) (RocketChat#10560)
  Regression: Webhooks breaking duo to a too restrict test (RocketChat#10555)
  Fix issues with the rooms and apps (RocketChat#10559)
  Fix regression with announcement bar being displayed without content (RocketChat#10554)
  LingoHub based on develop (RocketChat#10545)
  Regression: Revert announcement structure (RocketChat#10544)
  Regression: Upload was not working (RocketChat#10543)
  Remove duplicated key from en.i18n.json
  Included missing lib (RocketChat#10532)
  dependencies update
  ...
@rodrigok rodrigok mentioned this pull request Apr 28, 2018
@geekgonecrazy
Copy link
Member

@kaiiiiiiiii which oauth plugin are you using?

https://wordpress.org/plugins/oauth2-provider/ this plugin seems to not have /oauth2 but /oauth as the path.

Also then when changing it to /oauth manually in the url, then this: An+unsupported+scope+was+requested error is returned

@cardoso
Copy link
Contributor

cardoso commented Apr 30, 2018

@geekgonecrazy I think it's not a plugin, but the built-in (first-party) Wordpress Application OAuth 🤔

At least I didn't have to use any plugins to set it up.

https://developer.wordpress.com/docs/oauth2/

@kaiiiiiiiii
Copy link
Contributor Author

kaiiiiiiiii commented Apr 30, 2018

@cardoso Yipp, you’re right :)

@geekgonecrazy There’re a bunch of different oauth plugins and all of them work in a different way. I thought the RC built-in Wordpress oAuth was meant to be for wordpress.com/.org (+ JetPack)?

@geekgonecrazy
Copy link
Member

We are on-premises so most software also is on-premises for our users. At least we try to be friendly to that. So maybe we need a drop down in the wordpress option to select between wordpress.com and plugin based?

Or... we need to make it clear that this is now for wordpress.com. As previously was not.

Most that I know of using it are using a plugin with wordpress hosted by a Cpanel type shared hosting setup. Not many that I know of use wordpress.com

@cardoso
Copy link
Contributor

cardoso commented May 1, 2018

I think it needs to be a separate option with the name of the plugin, instead of just "Wordpress" @geekgonecrazy as I was completely unaware that it was a plugin.

But we shouldn't be committing to maintaining an integration of a third-party plugin of a third-party service in our main codebase IMO. (Which also implies we need to maintain it in both Android and iOS)

Maybe this can be a Rocket.Chat App in the future? 🤔

@kaiiiiiiiii
Copy link
Contributor Author

kaiiiiiiiii commented May 1, 2018

Thanks for pointing that out @geekgonecrazy and sorry for any inconvenience.

But as @cardoso already said, is it a good idea to maintain a 3rd party plugin? I think 3rd party plugins can still be managed trough the custom oauth apps?

Anyways, I can work on a better solution (I like the dropdown idea) the next couple of days.

trongthanh added a commit to goalifyplus/Goalify.Chat that referenced this pull request May 2, 2018
* goalify: (104 commits)
  Regression: Various search provider fixes (RocketChat#10591)
  Supplement TOS and privacy policy texts for use within server instances
  Fix /api/v1/settings.oauth not sending needed info for SAML & CAS (RocketChat#10596)
  Fix the Apps and Livechats not getting along well with each other (RocketChat#10598)
  [FIX] Missing "Administration" menu for users with some administration permissions (RocketChat#10551)
  [FIX] Member list search with no results (RocketChat#10599)
  merge vi-VN and vi json
  Add and enhance translations
  Update gitlab, npm package lock, include current server update script
  Adds Visual Studio Code debugging configuration (RocketChat#10586)
  [FIX] Integrations with room data not having the usernames filled in (RocketChat#10576)
  fixed problems with margin negative (RocketChat#10558)
  Add some information regarding Zapier and Bots to the integrations page (RocketChat#10574)
  Added target="_blank" to homepage and support link. (RocketChat#10575)
  [FIX] Stop Firefox announcement overflowing viewport (RocketChat#10503)
  [FIX] Wordpress oAuth authentication wasn't behaving correctly (RocketChat#10550)
  Fix inconsistent response of settings.oauth endpoint (RocketChat#10553)
  Regression: Remove added mentions on quote/reply (RocketChat#10571)
  Fix the attachments and fields incorrectly failing on validation (RocketChat#10573)
  Deps update (RocketChat#10549)
  ...
@geekgonecrazy
Copy link
Member

geekgonecrazy commented May 2, 2018

I'm not sure if long term it is a good idea. But for sure just changing does break those that already use it with wordpress. And no worries, we approved and merged. I saw the PR but didn't think anything of it.

@RocketChat/core thoughts? What's the best way to handle this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
0.64.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

8 participants