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

[3.4] [Updated] Users invitation #6220

Closed
wants to merge 11 commits into from
Closed

[3.4] [Updated] Users invitation #6220

wants to merge 11 commits into from

Conversation

GwendolenLynch
Copy link
Contributor

@GwendolenLynch GwendolenLynch commented Jan 2, 2017

Closes #5496
Fixes #4555

This is a reworking of the PR done by @mangroud in #5496
This is not ready for merge yet , as I'd like to squash my commits … when we decide collectively on approach … just want to (mostly) get this off my plate.

A UX addition in this one too:
screenshot from 2017-01-02 18-21-39

@GwendolenLynch GwendolenLynch added this to the Bolt 3.4 - Feature release milestone Jan 2, 2017
@CarsonF
Copy link
Member

CarsonF commented Jan 29, 2017

What do you think about prefilling (possibly forcing) a username / email address / even display name with the invitation? Really the only thing I don't want to fill in for my users is their password. I'm not saying this should be forced from the admins POV.

@GwendolenLynch
Copy link
Contributor Author

GwendolenLynch commented Jan 29, 2017

What do you think about prefilling (possibly forcing) a username / email address / even display name with the invitation? Really the only thing I don't want to fill in for my users is their password. I'm not saying this should be forced from the admins POV.

Can we? My understanding was that the intention was to make this an admin feature … that said, yes I think we should either enforce it as required, or make the fields validate.

That said, one of my massive concerns about this from the beginning was the security implications, as anyone with a valid link could (until said link is used) create an account… so TBH, there should be some additional stop-gaps to attempt to curb abuse.

@CarsonF
Copy link
Member

CarsonF commented Jan 29, 2017

Yes it is. I guess my use case is basically the same as create user and then send them a reset password link...

@GwendolenLynch
Copy link
Contributor Author

Yes it is.

Derp, by "can we" I meant "is it implemented for non-admin users?"

I guess my use case is basically the same as create user and then send them a reset password link...

This is pretty much where I am going. While it is a small vector, it is one that I can see potentially ending up in a CVE if we're not careful.

@bobdenotter
Copy link
Member

Does this need anything else than monkey-testing?

@GwendolenLynch
Copy link
Contributor Author

Yes, @CarsonF is working on more changes currently

@GwendolenLynch
Copy link
Contributor Author

@CarsonF has a copy of this branch … I am withdrawing my work on it, if it gts over the line, it won't be one by me

@GwendolenLynch GwendolenLynch deleted the mangroud/users-invitation branch February 22, 2017 19:17
@zomars
Copy link
Contributor

zomars commented Nov 9, 2017

Is this feature dead? I got lost between issues and PRs.

@GwendolenLynch
Copy link
Contributor Author

Not dead, just lacking time to finish. 😞

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.

None yet

5 participants