Skip to content

chore: Move scope into the same auth call #4162

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

Merged
merged 5 commits into from
Sep 23, 2022
Merged

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Sep 22, 2022

What this does

Does the scope authorize alongside the regular roles, rather than calling Authorize twice.

Comment on lines +147 to +170
if len(results) == 1 && len(results[0].Bindings) == 2 {
roleCheck, ok := results[0].Bindings[rolesOkCheck].(bool)
if !ok || !roleCheck {
return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), input, results)
}

return nil
scopeCheck, ok := results[0].Bindings[scopeOkCheck].(bool)
if !ok || !scopeCheck {
return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), input, results)
}

// This is purely defensive programming. The two above checks already
// check for 'true' expressions. This is just a sanity check to make
// sure we don't add non-boolean expressions to our query.
// This is super cheap to do, and just adds in some extra safety for
// programmer error.
for _, exp := range results[0].Expressions {
if b, ok := exp.Value.(bool); !ok || !b {
return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), input, results)
}
}
return nil
}
return ForbiddenWithInternal(xerrors.Errorf("policy disallows request"), input, results)
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed the query to 2 bindings, so can't use the easy Allowed() anymore 🤷

@Emyrk Emyrk requested a review from johnstcn September 23, 2022 14:51
@Emyrk Emyrk marked this pull request as ready for review September 23, 2022 14:51
@Emyrk Emyrk requested a review from sreya September 23, 2022 14:51
@Emyrk Emyrk merged commit 2e30d05 into main Sep 23, 2022
@Emyrk Emyrk deleted the stevenmasley/merge_scope_rego branch September 23, 2022 15:07
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