Skip to content

Add select account parameter for google authorization - #1063

Merged
ssddanbrown merged 10 commits into
BookStackApp:masterfrom
justein230:master
Nov 10, 2018
Merged

ssddanbrown merged 10 commits into
BookStackApp:masterfrom
justein230:master

Conversation

@justein230

Copy link
Copy Markdown
Contributor

Useful for choosing an account if a default account is outside the scope of a G Suite organization.

Useful for choosing an account if a default account is outside the scope of a G Suite organization.

@ssddanbrown ssddanbrown left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @justein230 for creating this pull request.

I'm not confident if forcing this functionality for all Google sign-in users is a good idea. I think many people may enjoy a one-click experience which, as far as I can see, this would remove.

Comment thread app/Auth/Access/SocialAuthService.php Outdated
@justein230

Copy link
Copy Markdown
Contributor Author

@ssddanbrown I added an env switch for this called GOOGLE_SELECT_ACCOUNT, and I check if it is true before adding that parameter. Let me know if this is sufficient for this feature to be added.

@ssddanbrown ssddanbrown added this to the BookStack Beta v0.24.2 milestone Nov 4, 2018
@ssddanbrown
ssddanbrown changed the base branch from master to v0.24-dev November 4, 2018 15:24
@ssddanbrown
ssddanbrown changed the base branch from v0.24-dev to master November 4, 2018 15:24
Comment thread app/Auth/Access/SocialAuthService.php Outdated
@justein230

Copy link
Copy Markdown
Contributor Author

@ssddanbrown I made the requested changes, and can't figure out why the checks have failed. I tested in my development environment and the changes work as intended. Any ideas?

@ssddanbrown

Copy link
Copy Markdown
Member

@justein230 Looks good. The tests being broken was totally my fault. I'd left some broken bits in master branch for a while so nothing to sorry about with your code. Will merge in now for the next release.

@ssddanbrown
ssddanbrown merged commit 4be0c56 into BookStackApp:master Nov 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants