Variable values should not be stored in Udaru

[dberesford]

Let's revisit variables; what we need to support is contexts in PBAB (which used to be called variables, but terminology was updated to match https://github.com/monken/node-pbac).

At it's most basic, these contexts are defined in policies, and their values are passed as parameters to pbac.evaluate, see the PBAC example.

The mistake we made with the current variables implementation is that we also store the values in Udaru, in what we're calling policy instances. Instead I think we can two sets of variables:

1) a fixed number of udaru specific variables, e.g. $user, $team, $org, and the values for these passed down through Udaru for every request automatically

2) all other variable values should be passed down from outside, i.e. /authorization/access/ -> iam.isAuthorized -> pbac.evaluate

Where the variable values for 2 are stored is not Udarus problem; they can be stored fully externally or there's always the option of using the metadata field.

I also suggest we move the terminology from variable to context like pbac have done.

Big change this, let's discuss.

[cianfoley-nearform]

Hi Guys,

I think Damien is right, we just need system variables like udaru.user, and udaru.teams (potentially an array because a user can belong to multiple teams.

e.g. if a policy is created with a variable

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": [
        "Read"
      ],
      "Effect": "Allow",
      "Resource": ["xyz:${udaru:teamId}"]
    }
  ]
}

Custom variables in the metadata is a good idea and could potentially used in two ways, i.e. to fill in a policy as with system variables and also to potentially grant access view/modify/edit certain teams (e.g. a policy might be set up to allow all teams marked with a blue tag read access to each other)

Any thoughts?

C.

[paolochiodi]

To add some context, current variables system serves a different use case.

We wanted to create a system that would allow the creation of "policy templates" and variables allows for customisation of such templates. We need to store those variables because they are part of the policy definition and can't be inferred when checking authorisation.

As an example, consider a policy that allows to edit a team. If we have 100 teams, we would need to create 100 different policies, each giving edit rights to a single team. With template policies we can instead create a single policy "edit X team" and then define X through variables when assigning the policy. X can't be inferred when requesting authorisation.

This variable system was designed to address a specific customer requirement to simplify policies creation and make the more re-usable - we should check if that's still a requirement or if we can drop it.

[dberesford]

Hi @paolochiodi, thanks for the context on that. We had a quick meeting about this last week. The feeling at the time was we still need to store contexts (variable values) in Udaru, but I still challenge this; for every example I've seen so far of template policies, the variable values can be passed down from the request, I haven't seen a case yet where we need to store them in Udaru.

Furthermore, AWS doesn't have a feature where you can store variable values; https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_variables.html

You can do this by using policy variables, a feature that lets you specify placeholders in a policy. When the policy is evaluated, the policy variables are replaced with values that come from the context of the request itself.

Still open for discussion, but I'm still of the opinion above; let's remove template instances, support contexts (via PBAC), have our fixed set of Udaru context values ($user, etc), and everything else is passed in form outside. I believe that this will still support the customer requirement to simplify policy creation.

[mihaidma]

Important to emphasise here that we need to support policies with variables (not relevant if implemented with context or template instances) as this is the main way to make simple, reusable and manageable policies.

[mihaidma]

Indeed Amazon does not seem to store context related to possible external variable values. All their samples have values provided by the internal Amazon context, stuff like "amazon userId", "amazon account number". We also have this type of internal context in which we have org id and user id, possible also team id in some cases. We can pass this to pbac easily.

Let's discuss on a sample case to clarify what we could do about the external context.

We have a policy:

id: docPolicyId
Action: 'read'
Resource: '${docId}'

and we want a user to be part of the team named 'team1' that gives members the rights to read 'doc1' and 'doc2'

We store in DB the information that 'team1' has the 'docPolicyId' attached and we store somewhere else the information that for that policy the team can have as context 'doc1' or 'doc2' for $docId. Also in DB we store info that the user is a member of 'team1'.

• in the case the context related info is stored on the client side, the check access request should be: "is the user allowed to read doc3?" but we also have to send info that "user has a relation to team1 that has in context docId with values 'doc1' and 'doc2'", or something along this line. This seems to duplicate relations already stored in Udaru beside the fact that the client app needs to understand some Udaru relations stuff.
• in the case we store in Udaru the context in 'team1' metadata, we would need to store there two entries saying the policy id and the two context values (at evaluation time looks like two calls to pbac). Then if we want to remove 'doc1' from the context we have to operate on the metadata json. This starts feeling like the 'instance' implementation that we have now only that we put the context in tables as two separate records (that could be very well be one json). The metadata field will be used also by client solutions to store their specific data and this raises access and security concerns.

What am I missing here? What other options do you see?

[paolochiodi]

The problem that makes template policy and the current variables system not portable to standard pbac variables is the fact that variables value can't be and shouldn't be inferred from context or user input. The variables are used to determined the admin intention.

To reuse my previous example with a policy to allow edit team X, imagine we have three teams: team1, team2 and team3 and we want to assign a policy to team1 to allow edit of team2 and a policy to team2 to allow edit of team3.

Prior to template policies and variables system we would have to create two policies:

• allow edit team2 and assign it to team1
• allow edit team3 and assign it to team2

Template policies were created to simplify this scenario, so you can now create a single policy allow edit X and assign it to team1 with X=team2 and to team2 with X=team3. Since value of X is inferred at "assign time", this scenario works exactly as the previous one.

If we use pbac/aws style variables, we would create policy allow edit X and assign it to team1 and team2. When checking for authorisation we would need to infer X, but we can't use the context nor the user input. Let's say we want to check if team1 can edit team3 (it shouldn't). If we infer X from context, we could only use current team, team1 so allow edit X would translate to allow edit team1 which is not what was initially intended by the admin (and could result in security issues). If we infer X from user input allow edit X would translate to allow edit team3. This would grant permission, even if it wasn't what the admin intended. This is even more dangerous, as such a policy with X inferred from user input will always authorise edit on a team.

The only way to capture admin intent is to define variables value when assigning template policies, not at "authorisation time".

I think the confusion arise from the fact that we call variables both the template policies and the pbac context. They are really two different features that serve two different use cases.

We can have

1. template policies, used to reduce duplication in policies
2. pbac variables, used to involve context into the policies

[paolochiodi]

All that said, I'm happy with removing or changing the template policies feature if it's too obscure (and if it isn't required by one of our customers).

I just wanted to point out that it can't be replaced by pbac variables/context.

[mihaidma]

@paolochiodi i did no see any contradiction between what i said and what you said.

The only difference i see is that I have the impression that we can pass beside the internal context also the what we called "variable values" as external context although technically this is not a runtime context but something saved when assigning the policy.

[cianfoley-nearform]

@paolochiodi case makes sense, it's a kind of policy inheritance but I think they should be managed globally and assigned to teams, users and orgs as you would any other policy.

Consider this scenario:

If we had a master policy in this case, as Paolo defined (Manage Team X, with id policyId1), and could then define two child policies (Manage Team 2 , and Manage Team 3) supplying fixed values for those variables, we could assign these child policies their own policy Id that could be used at any level e.g. policyId2 and policyId3 for Manage Team 2 and Manage Team 3 respectively.

After assigning Manage Team 2 and Manage Team 3 their policy Ids and can be applied to Team 1 and Team 2 respectively using the standard add policy functionality without storing the variables in the team_policies tables and can be applied at any level.

Storing the custom policy variables at the org, team, user level like this would mean we would need to add the policies with the same variables again if we wanted team4 to be able to access team 2 and team 3 for example, whereas if we define them globally, we could now apply Manage Team 2 and Manage Team 3 policies, which inherit from master, to team 4 using the ids policyId2 and policyId3.

This would further reduce administration burden.

Maybe the same could be achieved with context and conditions but I don't think it can.

Saying that we still need the context stuff too for variables in resources and conditions.

In terms of the context variables/condition variables, which are different but could yield similar effects if we use custom variables we may need to be able to do both of the following...

Self Authorization if a user is checking if they have authorization to resourceY themselves and we use the users own context then template policies referring to context variables should work once those system variables are applied... if we need other more specific variables they could be supplied to context via metadata up the chain from user through their parent teams... this sort of tagging is done in amazon, we could use path to metadata for this e.g. udaru.user.metadata.x.y.z and if present apply.

Third Party Authorization Checks If it's a third party app asking on behalf of a user then the context would need to be loaded at evaluation time too, there may be use cases for this on an admin panel to check other users rights, what policies are applied and see all variables that will be applied too and if present in users metadata etc. this stuff would need to be rendered visually so admins can understand it fully in action and keep tight control.

[paolochiodi]

@cianfoley-nearform I agree that the same feature could be implemented different and better ways.

I just wanted to clarify the original use case for the current implementation and point out the fact the pbac context/variables can't fully support it.

With regard to your proposed implementation of the same feature it looks like a lot similar to just creating multiple policies without using template policies. In an ipotetical user interface, the user would still need to create the child policy and when assigning it would still need to scroll through a long list of policies. By defining variables value on assign we don't require the intermediate step of creating a child policy and we reduce the list of policies to scroll through to choose the correct one.

[cianfoley-nearform]

@paolochiodi, understood, maybe once created it could be added to a pool of specialised policies that can be assigned to others? if another user tries to create the same one it could be flagged easily (it would have the same checksum for example). If we store the id globally with other policies, it's just a slightly different type of policy... the main reason i'm suggesting this is that we have policy instances now being managed in 3 separate tables in the db and would need to repeat same management code in endpoints for org, teams, users.

[dberesford]

Ok, in an attempt to summarise, we are proposing we need three context variable values: 1) system context values, supplied by udaru for all policies, $userId, $teamId, $teamPath etc 2) user context values, supplied at authorization time, e.g. $docId or something 3) admin context values, supplied at administration time, e.g. allow edit $X, where X is defined by an admin

1 & 2 we should do, no question.

If we do 1& 2, 3 to me I'm still not convinced it's worth the effort/complication, I've read the threads above and I can't help but feel we're over engineering this. @mihaidma can you PM me code links to the two real world projects that use template policies? I want to review to see how they would be re-architected assuming we had 1&2.

Thanks all for your time and contribution to this discussion.

[dberesford]

Update on this from our meeting: 1) system context values - yes, let's do this first 2) user context at auth time - this is a big security hole, let's not do it 3) admin context values - this is the replacement for policies instances we have currently - more thought & design needed here but this is the direction we want to go.

[cianfoley-nearform]

Ok guys, I've had a chance to review the policy code and here's a suggested solution, which should cover most of it off:

In listAllUserPolicies (packages/udaru-core/lib/ops/policyOps.js) we have a large sql statement retrieving all policies associated with a user including teams (traversing up their team path(s)), orgs and of course their own... this will need to be reworked to remove all traces of variables.

Aside: The endpoints to add variables to policies would then be modified to just add policy ids (affects users, teams and orgs). Delete is not affected as it just takes policyid as param. Context will be added later and associated just as metadata is.

After the sql is run it is mapped to iam policy rows in packages/udaru-core/lib/mapping.js mapIamPolicy function, which subsequently calls compileStatements to essentially replace the template variables with the values in the policy instances (the relationship to the team is intrinsic here which is important point as you'll discover in a min)

We could substitute the user, team and org context variables here in compileStatements but we've lost the context for teams without the team id here, so we either need to store the entity ids and types with the policy records for evaluation, or work it out as we're retrieving from db... i think a compile is better so we will need to store against each policy relationship record. it might be worth moving the compile then to the authorize module in a function called applyContext or something like that prior to iam authorize call rather than map straight away after the sql is executed.

The other issue then is if teamId variable is used in a policy that is assigned to a user, what teamId would be applied at evaluation?, if all parents then complexity is added (e.g. do we apply all parents up the chain, direct parent, path?), or we could simply NOT evaluate team variables at user level.

In terms of context variable substitution, I suggest we stick to entity ids for now and define the following variables for use with the prefix udaru: udaru.userId udaru.teamId udaru.organizationId

Potentially name properties could be used by might be best to use custom context variables for that and to implement that suggest we use the convention udaru.context.[variable], which are associated with teams, users and orgs, similar to the metadata addition, i.e. optional JSONB fields. At the authorize step where we replace id variables, we would then attempt to replace context variables if discovered in the policies and they have been assigned to the entities context.

@dberesford @mihaidma @paolochiodi, if this sounds OK I will proceed...

-EDIT-

Suggest that our evaluate be modified to supply the context for variable evaluation...

we have the resource we're looking to evaluate against so if we create the context object we should not need to find and replace variables as we are doing in the compile statement... PBAC should do that for us... (however, we still need to know what context to apply against a policy for the example Paolo gave)...

e.g.

pbac.evaluate({
  action: 'read',
  resource: 'resource:testTeam',
  context: {
    udaru: {
      teamId: 'testTeam',
    }
  }
});

[cianfoley-nearform]

closing this issue as we have context variables now supported.

we kept policy instances and added an instance id field to support individual deletion, if we implement a solution for context for teams (the main problem now would be evaluating which parent teams context to apply and what to do in the case of variable clashes) it may make policy instances redundant in the future but for now, the way the system works, they are important.