Skip to content

Conversation

@vtran99
Copy link
Contributor

@vtran99 vtran99 commented Aug 13, 2024

Fix this case error in private form:
managers are denied access when registering users in the management area.

@ThiefMaster
Copy link
Member

Have you tested this PR? self.management is not defined in many places, so this breaks other places such as accessing the registration form in the normal way (outside management)...

Also, in the place where the access error actually happens (the endpoint that checks the email address of the registrant), self.management is provided by the client and does not contain any information whether the user actually has management privileges (this is fine because the endpoint only returns information and management vs regular determines if certain validation errors are errors preventing the registration or just warnings that can be ignored).

@vtran99
Copy link
Contributor Author

vtran99 commented Aug 13, 2024

I use "self.management" because I presume the checking for management rights is already done in other places (this code check access had been added only for the private form)...
So I can change
either with
not getattr(self, 'management', None) either with
not self.event.can_manage(session.user, permission='registration')

For me, the check permission registration should have be done in other places...

@ThiefMaster
Copy link
Member

The only place where self.management is used is in classes that inherit from RegistrationEditMixin. And those classes set it on the class level - the display/management RHs have different inheritance and thus get the access checks depending on their base classes.

Anyway, I'm doing the same for the email checker now, ie having two endpoints, one in management and one in display, which avoids any "weird" manual access checks. See #6486 for my PR and feel free to give it a try! :)

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.

2 participants