Closed jpd236 closed 5 months ago
I would love if we could make progress on those issues, but I don't want to put a bunch of roadblocks in front of you. As a compromise, could we hide the Google account login behind a feature flag? That way it can be off by default to avoid those interactions, but you can turn it on for your deployment. And if #296, #631, and #698 ever get fixed, I'd be very happy removing the feature flag and making this behavior the default.
It makes sense that we're not ready to let arbitrary accounts be created. But I think there may still be at least a little bit of utility in having the Google sign-in button for existing users who have associated their Google Accounts so you don't have to worry about the username/password entry. So I kept the login button, but now we throw an error if the Google Account doesn't match that of an existing user. Does this seem worth keeping for you, or would you rather I just go ahead and put it behind a flag instead?
(FWIW, my current plan for the follow-up is to allow configuration of an optional Drive folder that will be used for ACL checks, and to only create new users in JR if they have access to that folder. Then I'll need to decide whether that should suffice for access to all hunts, or if we want to try to keep per-hunt ACLs working in this world).
Yeah I'm fine with "throw an error if the account doesn't exist already"
Just for the record, I know that your Drive folder thing is coming, and I haven't decided how I feel about it yet 🙂. I guess I need to get on that.
Sorry again for not getting a chance to look at this sooner. This seems like a good compromise approach for now, and the implementation seems good to me.
I did have one question around your specific requirements. If I'm reading this correctly, it's not possible to create a new JR account via Google sign-in, right? Only log into an existing account? And looking at #2056, it looks like the invite links currently require you to already have an account to apply an invite. Do you have a flow in mind for how your users create their accounts initially?
Ah. I just read your comment in the description of #2056 as reserving that for future work. In that case, I'm happy with this
Show a "Sign in with Google" button on the login page. This sign in flow first attempts to match against existing users who have linked that same Google Account. If there is no such user, a new account is created.
Icon taken from: https://developers.google.com/identity/branding-guidelines
Notes / issues:
This allows anyone to create a Jolly Roger account as long as they have a Jolly Roger account; previously, access was invite only. This seems reasonable in that hunts have their own ACLs, but if this is a concern, we could reject new account creation until a follow-up change which optionally configures access based on Drive folder access.
A user can unlink their Google Account; if the user was originally created through this flow, they will be left with an account with no password or method of entry. However, they can follow the forgotten password flow to create a new password (if an email server is configured).
At least on local instances, a console warning appears upon showing the Google Account popup: "Cross-Origin-Opener-Policy policy would block the window.closed call". This also occurs for the existing Google Account integration so isn't exclusive to this flow.
If the popup is closed without completing sign-in, the button remains disabled because we don't appear to get a failure callback. I'm unsure why this is happening, though it's easy enough to refresh the page.
Sometimes, when you click Sign Out, the login page renders with a blank "Sign in with Google" button, though the button appears to work as expected.
See #2021