search

fablabbcn / fablabs.io

The platform of the global Fab Labs Network
https://fablabs.io
GNU Affero General Public License v3.0
67 stars 33 forks source link

OAuth API error when trying to login for external apps #587

Closed MacTwister closed 2 years ago

MacTwister commented 2 years ago

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 2 years ago

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

viktorsmari commented 2 years ago

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 a1782ea9027959832aca61f46bd1a56948c8162b

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:

viktorsmari commented 2 years ago

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 2 years ago

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.

MacTwister commented 2 years ago

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