-
Notifications
You must be signed in to change notification settings - Fork 27
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
Parse UUID from auth header for anonymous users #123
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need checks for UUID collisions since they are being generated client-side? I suppose at our current scale the possibility is extremely unlikely and even if it does happen, it won’t be a significant event
Hmm.. unless there is a bug, uuids should not collide. 🤔 I don't think we will be able to detect collisions easily. 🤔 We would have to add some special header or flag when a UUID is generated, and check for uniqueness at backend side. 🤔 🤔 Or hope that such a bug will never happen. If it does, anonymous users would start managing each other's queue. What do you think? |
Collisions depend on the nature of randomness and the version of the uuid
generator. But I agree that since detecting and handling such collisions
will be troublesome we don’t need to put in effort there.
Besides, any user who doesn’t sign in needs to accept the risks associated
with anonymous queues.
…On Sun, 21 Feb 2021 at 6:47 PM, daltonfury42 ***@***.***> wrote:
Do we need checks for UUID collisions since they are being generated
client-side? I suppose at our current scale the possibility is extremely
unlikely and even if it does happen, it won’t be a significant event
Hmm.. unless there is a bug, uuids should not collide. 🤔
I don't think we will be able to detect collisions easily. 🤔 We would
have to add some special header or flag when a UUID is generated, and check
for uniqueness at backend side. 🤔 🤔 Or hope that such a bug will never
happen. If it does, anonymous users would start managing each other's queue.
What do you think?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#123 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADBPWPFD4SDR6YVAKBZNGFDTAEBWZANCNFSM4X66K64A>
.
|
|
||
@Test | ||
void denyBadHeaderValues() throws MalformedURLException { | ||
var authFilter = new AuthenticationFilter(new LoggedInUserInfo(), keyUrl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A unit test should test only one unit of work. Here you test 6. So we can use @ParameterizedTest
annotation and the method will look like this:
@ParameterizedTest
@ValueSource(strings = {null, "", "invalid header"}) //and so on
void denyBadHeaderValues(String headerValue) throws MalformedURLException {
var authFilter = new AuthenticationFilter(new LoggedInUserInfo(), keyUrl);
assertThrows(SQAccessDeniedException.class, () -> authFilter.authenticate(headerValue));
}
See Related to SimplQ/simplQ-frontend#477 for details.
Users who have not signed in will send to the backend a UUID generated at client side. This is used to identify the user, till he chooses to register.
Frontend PR: SimplQ/simplQ-frontend#554