Allow apps to restrict indicators - #558
Conversation
Jira: IAM-1989
Yes, this uses a different linter than the project officially does. But, it also happens to make things clearer. Jira: IAM-1989
f61645a to
1c2e673
Compare
| @@ -310,20 +348,22 @@ exports.onExecutePostLogin = async (event, api) => { | |||
| ) { | |||
| console.log(`${event.user.user_id} was in authorized_users`); | |||
| risk = app.AAL || risk_default; | |||
There was a problem hiding this comment.
Lines 350 & 359: Does this assign the stricter indicator? Seems like, whatever rule is evaluated last wins, and there is no comparison against the previous value.
There was a problem hiding this comment.
I'm about to enforce that all applications have the same AAL / Risk. I think that allowing applications to have multiple levels of risk will confuse us when it comes to auditing. Admittedly, it's really the app-group association we end up caring about, and less so about the app itself (e.g. group X has elevated privileges in app, hence the potentially increased risk).
But, this doesn't happen often enough at Mozilla for us to have a policy around it. So here we make one for ourselves: don't support multi-risk apps.
There was a problem hiding this comment.
There was a problem hiding this comment.
But for assigning the more strict indicator: that's what the maxIndicator function does.
We take in the previous indicator (stashed in indicator_required, which may be undefined) and the app's indicator (app.AAI), and take the max of the two (or fall back to a default).
Follow-ups required: * Extend the schema in sso-dashboard to support AAI; * Extend the schema in sso-dashboard-configuration to support AAI; * Add a test in sso-dashboard-configuration to assert each application has only one risk level. That last one was weird, so here's why: because it's just sort of easier from a governance perspective. We could support multiple risks for the same app (and optionally different groups), but that makes auditing a bit tougher. (e.g. is OpenAI HIGH or MEDIUM? Depends on the group? Nah.) Jira: IAM-1989
1c2e673 to
1cbcab9
Compare
We only ever add _one_ additional factor to the JWT. For non-LDAP folks we don't use Duo, but for LDAP we do. We, by the end of this series of commits, will want to change our logic based on the indicator/factor required. Jira: IAM-1989
gcoxmoz
left a comment
gcoxmoz
left a comment
There was a problem hiding this comment.
I've kinda gone cross-eyed at this one but I'm pretty much at the point of "you've cleaned the language and it's mostly making sense" so I don't think I have quibbles enough to hold you back on.
3 participants
Todo:
AAIAAIAAI