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 Relative Approots non intrusive #178

Merged
merged 6 commits into from
Oct 31, 2023

Conversation

cptrodolfox
Copy link
Contributor

@cptrodolfox cptrodolfox commented Oct 31, 2023

By using an alternative with oauth2RedirectUri users of the library can set up their own redirect URIs.

As part of Stack Builders Inc. Hacktoberfest 2023 event.

Copy link
Member

@pbrisbin pbrisbin left a comment

Choose a reason for hiding this comment

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

This almost feels too easy!

Let me make sure I understand...

Previously,

Providers would create an OAuth2 with oauth2RedirectUri set to Nothing, and this function would then build a callback using PluginR and set it to that. This relies on getParentUrlRender which relies on approot being non-relative.

Now,

Providers may set an oauth2RedirectUri ahead of time and it will be respected.

This allows an app that uses a relative approot to function, as long as they use providers that are handling oauth2RedirectUri themselves, and ensuring it's absolute through whatever provider-specific variables they may accept.

This seems kind of niche to me, but it's very non-intrusive and totally opt-in, so sounds great!

@cptrodolfox
Copy link
Contributor Author

@pbrisbin I have done some code changes to avoid calling the getParentUrlRender if the oauth2RedirectUri is set.

Copy link
Member

@pbrisbin pbrisbin left a comment

Choose a reason for hiding this comment

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

LGTM. I will merge/release once I see it's Green.

@cptrodolfox
Copy link
Contributor Author

LGTM. I will merge/release once I see it's Green.

Thanks for the feedback.

@pbrisbin pbrisbin merged commit 7d913b6 into freckle:main Oct 31, 2023
13 checks passed
@pbrisbin
Copy link
Member

pbrisbin commented Nov 1, 2023

Released as v0.7.1.3.

@DavidMazarro
Copy link

@cptrodolfox Thanks for the contribution! Remember to add the following line in the PR's description so that the contribution can be considered valid for Stack Builders' Hacktoberfest 2023 event:

As part of Stack Builders Inc. Hacktoberfest 2023 event.

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.

4 participants