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

SSO: Add redirect_uri to oAuth auth request #969

Merged
merged 3 commits into from
Dec 7, 2023

Conversation

Sapd
Copy link
Contributor

@Sapd Sapd commented Dec 4, 2023

Also:

  • Add client_id with the name of the app (for possible future use)
  • Add response_type simply to provide all required oauth2 parameters for reference
  • For the URL to be opened in Browser: Forward the callback/redirect_uri URL provided by the server
  • Use a hardcoded variable for HTTPS enforcement, for easier debugging

Also:
- Add client_id with the name of the app (for possible future use)
- Add response_type simply to provide all required oauth2 parameters for reference
- For the URL to be opened in Browser: Forward the callback/redirect_uri URL provided by the server
- Use a hardcoded variable for HTTPS enforcement, for easier debugging
Removed in the backend to increase compatibility with possible 3rd party oauth libraries people might use
@advplyr
Copy link
Owner

advplyr commented Dec 6, 2023

Can we make this backwards compatible with server v2.6.0 so that the app can be released before the server? Or we can release the server first and make that compatible with app v0.9.68.

@Sapd
Copy link
Contributor Author

Sapd commented Dec 7, 2023

Can we make this backwards compatible with server v2.6.0 so that the app can be released before the server? Or we can release the server first and make that compatible with app v0.9.68.

@advplyr

  • If the server is released first, it will expect a redirect_uri (which comes with this change)
  • If the client is released first, the server will expect a is_rest (which requirement will be removed with the server change)
  • Also I think a problem could be the change around line 201 which now takes the redirect_uri of the server, making it not compatible with older server versions (I think, bc in the old version it probably sent a link to the ABS callback url instead of the app one)

In theory one could postpone the isRest parameter (I removed it to show that the app just works with standard oauth parameters on the new server version). But the last point with the callback URL is the problem. If possible I would recommend releasing them at nearly the same time.

@advplyr
Copy link
Owner

advplyr commented Dec 7, 2023

The app has to go through a review process so it's difficult to release them at the same time. I'll see if I can get this backwards compatible

@advplyr
Copy link
Owner

advplyr commented Dec 7, 2023

I think that'll do it

@advplyr advplyr merged commit 76253b7 into advplyr:master Dec 7, 2023
1 check passed
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.

2 participants