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

Bug: Normalize both url path and capability substring for endpoint authorization #170

Closed
denopink opened this issue Jan 19, 2023 · 4 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@denopink
Copy link
Contributor

denopink commented Jan 19, 2023

bascule checks wether or not one of the sat token capabilities matches the request url. This works by checking 1) the capabilities like hooks is a subset match of the request url and 2) that match starts at index 0:

where re is the regex for hooks capability and urlToMatch should start with hooks

matchIdxs := re.FindStringIndex(urlToMatch)
if matchIdxs == nil || matchIdxs[0] != 0 {
return false
}
return true

The issue at hand is that go's net lib may include a leading / in its url path. So urlToMatch would point to /hooks instead of hooks and thus failing the endpoint authorization.

The solution is to normalize both re and urlToMatch to contain a leading / such that the currently logic works.

@denopink denopink added bug Something isn't working enhancement New feature or request labels Jan 19, 2023
@denopink denopink self-assigned this Jan 19, 2023
@denopink denopink changed the title Normalize endpoint regex authorization Bug: Normalize both url path and capability substring for authorization match Jan 19, 2023
@denopink denopink changed the title Bug: Normalize both url path and capability substring for authorization match Bug: Normalize both url path and capability substring for endpoint authorization Jan 19, 2023
denopink added a commit that referenced this issue Jan 19, 2023
@JC000
Copy link

JC000 commented Jan 19, 2023

The responsibility falls on the creators of the capabilities used in the authorization input to use proper regular expressions to contain word boundaries so that this doesn't happen.

@denopink
Copy link
Contributor Author

@johnabass I don't think we provide a way for a user to configure the regex for comparing these two strings.
Here's the code:

re, err := regexp.Compile(matches[1]) //url regex that capability grants access to
if err != nil {
return false
}
matchIdxs := re.FindStringIndex(urlToMatch)
if matchIdxs == nil || matchIdxs[0] != 0 {

If I'm not missing it and we're actually not supporting that functionality atm, then I can add it.
I believe JC has a good point since other anomalies may show up later.

@johnabass
Copy link
Contributor

johnabass commented Jan 19, 2023

It's not the regex a user would configure. It's that custom validators can be written to take advantage of what bascule has parsed. We already support that functionality, and we use it in our own software.

@denopink
Copy link
Contributor Author

closed by #173

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants