getfider / fider

Open platform to collect and prioritize feedback
https://fider.io
GNU Affero General Public License v3.0
2.75k stars 620 forks source link

[BUG] Callback URL is missing its scheme #1074

Closed asos-robbell closed 2 years ago

asos-robbell commented 2 years ago

Fider Cloud or Self Hosted v0.21.0

Describe the bug When an Azure AD OAuth provider has been configured, the callback URL that is visible in the Authentication section doesn't appear to include the scheme (https://). This causes issues when Azure AD attempts to complete the callback:

AADSTS90102: 'redirect_uri' value must be a valid absolute URI.

Note that the Azure portal will not allow the addition of a redirect URL that doesn't include a scheme.

This issue was not present In version v0.20.0, where the scheme is visible in the list of enabled OAuth Providers.

To Reproduce Steps to reproduce the behavior:

  1. Create an Azure AD OAuth provider as per your instructions
  2. Visit the Authenication section and observe that the Callback URL doesn't include the scheme
  3. Test the OAuth provider to see the message from Azure AD, above

Expected behavior Fider to include the scheme in the list of enabled OAuth providers and to pass on to the OAuth provider for callback to Fider.

Screenshots image

goenning commented 2 years ago

Does your BASE_URL starts with http://?

asos-robbell commented 2 years ago

Doing a little digging, this looks as though it may have been introduced by the move from HOST_DOMAIN to BASE_URL. Should BASE_URL be prefixed with the scheme?

asos-robbell commented 2 years ago

Does your BASE_URL starts with http://?

No—we saw the log message after upgrading that HOST_DOMAIN had been replaced by BASE_URL, but previously HOST_DOMAIN didn't include the prefix. Just redeploying now with the scheme included.

goenning commented 2 years ago

@asos-robbell did it work?

asos-robbell commented 2 years ago

@asos-robbell did it work?

It did. Do you normally publish migration details for moving between versions?

goenning commented 2 years ago

I'm using the GitHub releases to communicate breaking changes. Although I missed the part that BASE_URL should be a valid URL, we could probably add a validation on start up as well.