Skip to content

pkg/middleware: more robust sessionid handling#8

Merged
oxtyped merged 1 commit intooxtyped:mainfrom
sbinet:hmac-dot
Mar 25, 2023
Merged

pkg/middleware: more robust sessionid handling#8
oxtyped merged 1 commit intooxtyped:mainfrom
sbinet:hmac-dot

Conversation

@sbinet
Copy link
Copy Markdown
Contributor

@sbinet sbinet commented Mar 24, 2023

Dots '.' may appear in the decoded sessionid string. The logic to extract the username from the decoded sessionid string would then be defeated.

@sbinet
Copy link
Copy Markdown
Contributor Author

sbinet commented Mar 24, 2023

Perhaps a better way of handling this (trying to find where the username starts and where the key ends, is prone to errors: what if the username contains a . ?), would be to pass the expected username to the Verifier ?

Dots '.' may appear in the decoded sessionid string.
The logic to extract the username from the decoded sessionid string
would then be defeated.

Signed-off-by: Sebastien Binet <binet@cern.ch>
Copy link
Copy Markdown
Owner

@oxtyped oxtyped left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

}

parsedStr := strings.Split(string(decodedStr), ".")
i := bytes.LastIndexByte(session, '.')
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

@oxtyped
Copy link
Copy Markdown
Owner

oxtyped commented Mar 25, 2023

Perhaps a better way of handling this (trying to find where the username starts and where the key ends, is prone to errors: what if the username contains a . ?), would be to pass the expected username to the Verifier ?

I think this might be up for a separate discussion (maybe in a new issue). I'm not sure if we should support . in usernames since that might also have a bit of an effect when it comes to querying for usernames with . in the database iirc.

But should we choose either way, pkg/apis/handlers.go (https://github.com/oxtyped/gpodder2go/blob/main/pkg/apis/handlers.go#L50-L82) will still need to be updated to handle both scenarios.

@oxtyped oxtyped merged commit 4cb608f into oxtyped:main Mar 25, 2023
@sbinet sbinet deleted the hmac-dot branch March 27, 2023 11:38
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