| Summary: | Enable easily changing maintainer email address | ||
|---|---|---|---|
| Product: | [Yocto Project Subprojects] Layer Index | Reporter: | Paul Eggleton <bluelightning> |
| Component: | Layer Index | Assignee: | Unassigned <unassigned> |
| Status: | NEW --- | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | Medium+ | CC: | brian.avery, garbados, libertad.gonzalez.de.la.cruz, otavio, tim.orling |
| Version: | unspecified | ||
| Target Milestone: | 6.99 | ||
| Hardware: | x86 | ||
| OS: | Multiple | ||
| Whiteboard: | |||
| OS type for building Yocto: | --- | Type of Regression: | --- |
| Verified: | Documentation change: | No (bug/feature does not impact docs) | |
|
Description
Paul Eggleton
2017-03-21 03:56:00 UTC
Hello! I've got a WIP solution that uses a pre_save signal to update relevant LayerMaintainer objects when users change their emails. I'll add tests and then put it up for review. However, the LayerMaintainer model is implemented with its own `name' and `email' fields, so because user emails aren't unique among users, any user can change their email to that of a maintainer's in order to gain edit rights to their layers / branches. This seems like a security problem. It might be better to relate LayerMaintainer to a given User using a ForeignKey. This would prevent bugs like this, but would also imply changes to LayerMaintainerForm, and probably an update to (subclass of?) the User model to ensure emails are unique. However how you'd like to proceed, I'm happy to implement. Hi Diana, thanks for helping out with this.
This is indeed a bit of a can of worms. The reason LayerMaintainer is just email+name rather than a foreign key to User is so that people don't have to create accounts to submit layers - you can just enter the layer information including the maintainer's details and you're done. It's conceivable that we could normalise it down to one record per unique maintainer so that there are fewer records to update, but we'd have to do that internally - I don't think we can mandate that maintainers register.
It's going to be a bit of extra work, but I suspect what we ought to do in order to make this secure is to start a process similar to password reset when the user changes their email, i.e. it has to be confirmed by email ("Someone, possibly you, has elected to change the email address on the account xyz..., click here to confirm"). Once it's confirmed we actually make the updates. We'd also block changing the email to the same address of any other existing account.
Does that sound like a reasonable approach?
That sounds like a reasonable approach, and seems related to 11197 (running asynchronous operations). I'll sketch a fix and post about it. Bulk move from 4.99 or 0.00 to 5.99 Related, when a new maintainer is added, it has to be done on each LayerBranch for every layer... this was quite a few manual changes to add Rob Woolley as a maintainer for the various meta-ros* layers (clearly a django query script could have made that semi-automated). Still valid. |