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

Adds Alias Login's to Develop #493

Merged
merged 8 commits into from
May 30, 2023

Conversation

AW0005
Copy link
Contributor

@AW0005 AW0005 commented Jan 12, 2023

Same instructions as the extension for setting up:
http://wiki.lorekeeper.me/index.php?title=Extensions:Alias_Logins

@itinerare itinerare added enhancement New feature or request needs review Pull requests that are pending community review labels Jan 13, 2023
@itinerare itinerare self-requested a review January 13, 2023 00:16
@Ne-wt
Copy link
Contributor

Ne-wt commented Jan 13, 2023

might be a good idea to squash these commits to remove the original hard coding of TH (use git --reset soft)

@itinerare
Copy link
Collaborator

I was planning on squash merging regardless for good commit formatting so it shouldn't be a trouble

@itinerare
Copy link
Collaborator

Noting here that I've looked over the code and just want to play around w/ it a bit before approving!

@@ -67,6 +67,16 @@
</div>
</div>
</form>

<h3 class="text-center mt-5 pt-2">Alternate Logins</h3>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check if any sites are allowed for login, and show/hide the header based on if there are some/none?

@AW0005
Copy link
Contributor Author

AW0005 commented Jan 16, 2023

Update made for hiding the headers!

@AW0005
Copy link
Contributor Author

AW0005 commented Jan 19, 2023

Just pushed an update for a fix for the twitter socialite provider that does require a little bit of additional setup that I added to the extension page as well:
http://wiki.lorekeeper.me/index.php?title=Extensions:Alias_Logins

The trouble is that OAuth1 version of socialite providers don't give a way to programmatically provide an alternate redirect url which this makes use of. Laravel Socialite however has a built in Oauth2 provider for twitter so this has updated twitter auth to be based on that. Tested regular auth as well as login, all seems to work fine.

I did also notice that tumblr seems to be the one other socialite provider we have that uses oauth1 - so may need to put a guard against that, since I don't see any other alternative from my quick googling, besides attempting to rebuild it with OAuth2.

@itinerare
Copy link
Collaborator

I think setting a check up for tumblr and noting that atm it's not a supported option should be fine...

@Ne-wt
Copy link
Contributor

Ne-wt commented May 30, 2023

used on a live site with a somewhat large userbase and all seems to be functional. I think this should be a priority for 3.0.0 as there has always been resistance to the usage of aliases (whilst optional aliases does address this somewhat, the convenience of OAuth is unmatched IMO)

@itinerare
Copy link
Collaborator

Good to hear it + agreed-- ty for testing!

@itinerare itinerare merged commit 61f5b50 into corowne:develop May 30, 2023
@itinerare itinerare added reviewed Pull requests that have received community review and are pending merge and removed needs review Pull requests that are pending community review labels May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request reviewed Pull requests that have received community review and are pending merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants