fablabbcn/fablabs.io

OAuth API error when trying to login for external apps

MacTwister opened this issue · 5 comments

MacTwister commented

Describe the bug
It has been reported that in the last 2 days, users have been unable to login through fablabsio (ie. Nueval, Fabcloud).

When the app redirects to the authentication URL, https://api.fablabs.io/oauth/authorize?response_type=code&redirect_uri=....etc..., the following error is thrown:

translation missing: en.doorkeeper.errors.messages.invalid_request.missing_param

Has anything been updated in the Doorkeepr library version or config?

To Reproduce

  1. Go to Nueval (make sure you are logged out)
  2. Click on login with fablabsio
  3. See error
pral2a commented

We'll look at it asap! I can confirm other apps log in such as https://make.works/ work well

viktorsmari commented

I can also log in to https://make.works/ using fablabs.io without issues.

The doorkeeper plugin was updated from 5.0.3 -> 5.5.1 on 2021-04-20
We deployed that change 2 days ago so this is probably the issue:
https://github.com/fablabbcn/fablabs.io/blame/78d33975c9b9a36bad211999e9bbccc6cfc1646d/Gemfile.lock#L153

We also have this (probably unrelated) commit updating Doorkeeper views a1782ea

Ideally we should write a test which is similar to what the Nueval login requires, so we can catch the error before deploying, so we don't run into this error again.

Doorkeeper changelog:
https://github.com/doorkeeper-gem/doorkeeper/blob/main/CHANGELOG.md

If I compare the 2 Doorkeeper applications, Nueval and MakeWorks, the only difference I see is:

  • Nueval: SCOPES EMPTY
  • MakeWorks: SCOPES user
viktorsmari commented

Fixed by downgrading Doorkeeper - but we should keep this issue as a reminder that we need some kind of test to prevent breaking Nueval login in the future.

MacTwister commented

Please in future if you make a change to the Fablabsio login system, which is used by other applications, it would be good to notify the network of pending changes (or at least me for Nueval/Fabcloud). This way we can test the updates and synchronise any needed changes in our apps in a timely fashion, instead of now chasing a fix after 2 days of having a broken login on other apps.

  • 👍1
MacTwister commented

Ok thanks @pral2a & @viktorsmari for quick revert. Had a bit of time today to review this and indeed Doorkeepr will now require a scope to be defined for auths. Since this app does not use scopes, we can also define a default scope for requests that do not define one.

From upgrade guide: https://github.com/doorkeeper-gem/doorkeeper/wiki/Migration-from-old-versions#from-51x-to-52x