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

Change: Follow redirect convention for login also for developer flow #88

Merged
merged 5 commits into from May 3, 2021

Conversation

erenes
Copy link
Contributor

@erenes erenes commented Apr 17, 2021

Initially, to fix developer login flow I edited the client to support the different authentication flow offered by vanilla bananas-api, in short, when using github, the flow is:

> GET /user/authorize
< 302 https://github....etc
Client shows redirect url in commandline for copy-pasting.

While using developer flow, it is:

> GET /user/authorize
< 200 OK <login form>

My initial fix in bananas-frontend-cli detected a 200 response with html content and then showed the URL that was originally requested in the CLI for copy-pasting.

This PR changes the developer flow on the bananas-api side to:

> GET /user/authorize
< 302 <request-host>/user/developer?code=xxxx
Client shows redirect url in commandline for copy-pasting.

The original logon form is now hosted on the /user/developer url.

This allows an almost unmodified client to connect to the API. Unfortunately, in the client, the audience is hardcoded so some changes will still be required. Expect a new client PR to arrive soon :)

@erenes
Copy link
Contributor Author

erenes commented Apr 17, 2021

On a related note, how do I run the regression tests offline? 😅
Edit: nvm, figured it out :)

Copy link
Member

@TrueBrain TrueBrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nicely done, tnx. This is so much better indeed! Consistency between authz is very nice :)

(ps: sorry it took me so long to get to reviewing this ..)

bananas_api/user/developer.py Outdated Show resolved Hide resolved
@TrueBrain TrueBrain merged commit ee645e6 into OpenTTD:master May 3, 2021
@erenes erenes deleted the fix-dev-login branch May 3, 2021 15:13
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.

None yet

2 participants