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

Add support for 3LO loopback flow #132

Merged
merged 30 commits into from
Jun 28, 2022
Merged

Add support for 3LO loopback flow #132

merged 30 commits into from
Jun 28, 2022

Conversation

ulisesL
Copy link
Collaborator

@ulisesL ulisesL commented Jun 22, 2022

Implement logic to enable 3LO loopback.

util/loopback.go: contains the localhost-logic, which is in charge of handling the auth code and state.
util/browser.go: contains the logic to open the consent page in the OS default browser.
util/auth-handlers.go: makes use of loopback and browser to create the 3LO loopback algorithm.

@ulisesL ulisesL requested review from andyrzhao and shinfan June 22, 2022 04:41
Copy link
Collaborator

@andyrzhao andyrzhao left a comment

Choose a reason for hiding this comment

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

Thanks for the PR Ulises! This is a big change and I've left some comments regarding code organization - in general, lets move the new functions out of of main.go (into one or more util files as needed) to keep main.go as small as possible. (We can also think about adding co-located unit tests to some of the util functions in the future. Another reason to move it outside of main.go)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
integration/cli_test.go Outdated Show resolved Hide resolved
integration/cli_test.go Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
util/localhost.go Outdated Show resolved Hide resolved
util/cache.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@andyrzhao andyrzhao left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the previous comments! I've left some additional minor comments, should be good to merge the PR after that.

integration/cli_test.go Outdated Show resolved Hide resolved
util/auth-handlers.go Outdated Show resolved Hide resolved
util/auth-handlers.go Outdated Show resolved Hide resolved
util/auth-handlers.go Outdated Show resolved Hide resolved
util/clientId-id-file.go Outdated Show resolved Hide resolved
util/clientId-id-file.go Outdated Show resolved Hide resolved
util/clientId-id-file.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@andyrzhao andyrzhao left a comment

Choose a reason for hiding this comment

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

LGTM!

@andyrzhao andyrzhao merged commit e4d8719 into master Jun 28, 2022
@andyrzhao andyrzhao deleted the issue37 branch June 28, 2022 22:41
@andyrzhao andyrzhao changed the title Issue37 Add support for 3LO loopback flow Jul 19, 2022
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