Skip to content

Commit

Permalink
Merge pull request #821 from bosch-io/feature/policy-lockout-header
Browse files Browse the repository at this point in the history
Add header to optionally allow policy creation without WRITE access
  • Loading branch information
Stefan Maute committed Sep 28, 2020
2 parents ad72864 + ea62e4f commit 3a288f6
Show file tree
Hide file tree
Showing 17 changed files with 348 additions and 55 deletions.
45 changes: 37 additions & 8 deletions documentation/src/main/resources/openapi/ditto-api-2.yml
Expand Up @@ -117,6 +117,7 @@ paths:
- $ref: '#/components/parameters/requestedAcksParam'
- $ref: '#/components/parameters/timeoutParam'
- $ref: '#/components/parameters/responseRequiredParam'
- $ref: '#/components/parameters/allowPolicyLockoutParam'
responses:
'201':
description: The thing was successfully created.
Expand Down Expand Up @@ -2935,6 +2936,9 @@ paths:
### Create a new policy
At the initial creation of a policy, at least one valid entry is required. However, you can create a full-fledged policy all at once.
By default the authorized subject needs WRITE permission on the root resource of the created policy. You can
however omit this check by setting the parameter `allow-policy-lockout` to `true`.
Example: To create a policy for multiple coffee maker things,
which gives **yourself** all permissions on all resources, set the policyId in the path,
e.g. to "com.acme.coffeemaker:policy-01" and the body part, like in the following snippet.
Expand Down Expand Up @@ -2993,6 +2997,7 @@ paths:
- $ref: '#/components/parameters/ifNoneMatchHeaderParam'
- $ref: '#/components/parameters/timeoutParam'
- $ref: '#/components/parameters/responseRequiredParam'
- $ref: '#/components/parameters/allowPolicyLockoutParam'
responses:
'201':
description: The policy was successfully created.
Expand Down Expand Up @@ -3044,6 +3049,7 @@ paths:
* the caller has insufficient permissions.
You need `WRITE` permission on the root `policy:/` resource,
without any revoke in a deeper path of the policy resource.
(You can omit this check by setting the `allow-policy-lockout` parameter.)
content:
application/json:
schema:
Expand Down Expand Up @@ -4617,15 +4623,27 @@ components:
_copyPolicyFrom:
type: string
description: |-
This field may contain the policy ID of an existing policy.
The policy is copied and used for this newly created thing. This field may also contain a placeholder
reference to a thing in the format `{{ ref:things/[thingId]/policyId }}` where you need to replace `[thingId]`
with a valid thing ID. The newly created thing will then obtain a copy of the policy of the referenced thing.
In the case of using a reference, the caller needs to have READ access to both the thing and the policy of the thing.
In the case of using an explicit policy id to copy from, the caller needs to have READ access to the policy.
This field may contain
* the policy ID of an existing policy.
The policy is copied and used for this newly created thing. The
caller needs to have READ and WRITE<sup>*</sup> access to the policy.
* a placeholder reference to a thing in the format {{ ref:things/[thingId]/policyId }} where you need to
replace [thingId] with a valid thing ID.
The newly created thing will then obtain a copy of the policy of
the referenced thing. The caller needs to have READ access to the thing and READ and WRITE<sup>*</sup>
access to the policy of the thing.
<sup>*</sup> The check for WRITE permission avoids locking yourself out of the newly created policy. You can
bypass this check by setting the header `allowPolicyLockout` to `true`. Be aware that the authorized
subject cannot modify the policy if you do not assign WRITE permission on the policy resource!
If you want to specify a policy ID for the copied policy, use the policyId field.
This field must not be used together with the field `_policy`. If you specify both `_policy` and
`_copyPolicyFrom` this will lead to an error response.
This field must not be used together with the field _policy. If you specify both _policy and _copyPolicyFrom
this will lead to an error response.
policyId:
type: string
description: |-
Expand Down Expand Up @@ -5001,6 +5019,17 @@ components:
required: false
schema:
type: boolean
allowPolicyLockoutParam:
name: allow-policy-lockout
in: query
description: |-
Defines whether a subject is allowed to create a policy without having WRITE permission on the policy
resource of the created policy.
The default (if ommited) is `false`.
required: false
schema:
type: boolean


labelPathParam:
Expand Down
Expand Up @@ -330,6 +330,11 @@ public MetadataHeaders getMetadataHeadersToPut() {
return MetadataHeaders.parseMetadataHeaders(metadataHeaderValue);
}

@Override
public boolean isAllowPolicyLockout() {
return isExpectedBoolean(DittoHeaderDefinition.ALLOW_POLICY_LOCKOUT, Boolean.TRUE);
}

@Override
public JsonObject toJson() {
final JsonObjectBuilder jsonObjectBuilder = JsonObject.newBuilder();
Expand Down
Expand Up @@ -464,6 +464,12 @@ public S putMetadata(final MetadataHeaderKey key, final JsonValue value) {
return myself;
}

@Override
public S allowPolicyLockout(final boolean allowPolicyLockout) {
putBoolean(DittoHeaderDefinition.ALLOW_POLICY_LOCKOUT, allowPolicyLockout);
return myself;
}

@Override
public S putHeader(final CharSequence key, final CharSequence value) {
validateKey(key);
Expand Down
Expand Up @@ -259,7 +259,21 @@ public enum DittoHeaderDefinition implements HeaderDefinition {
*
* @since 1.2.0
*/
PUT_METADATA("put-metadata", JsonArray.class, true, false, HeaderValueValidators.getMetadataHeadersValidator());
PUT_METADATA("put-metadata", JsonArray.class, true, false, HeaderValueValidators.getMetadataHeadersValidator()),

/**
* Header definition for allowing the policy lockout (i.e. a subject can create a policy without having WRITE
* permission on the policy resource for itself, by default a subject making the request must have
* WRITE permission on policy resource).
*
* <p>
* Key {@code "allow-policy-lockout"}, Java type: {@link boolean}.
* </p>
*
* @since 1.3.0
*/
ALLOW_POLICY_LOCKOUT("allow-policy-lockout", boolean.class, true, false,
HeaderValueValidators.getBooleanValidator());

/**
* Map to speed up lookup of header definition by key.
Expand Down
Expand Up @@ -333,4 +333,12 @@ default DittoHeadersBuilder toBuilder() {
*/
MetadataHeaders getMetadataHeadersToPut();

/**
* Returns whether the policy lockout is allowed.
*
* @return {@code true} if the policy lockout is allowed
* @since 1.3.0
*/
boolean isAllowPolicyLockout();

}
Expand Up @@ -322,6 +322,15 @@ B acknowledgementRequest(AcknowledgementRequest acknowledgementRequest,
*/
B putMetadata(MetadataHeaderKey key, JsonValue value);

/**
* Sets the allowPolicyLockout value.
*
* @param allowPolicyLockout the allowPolicyLockout value to be set.
* @return this builder for method chaining.
* @since 1.3.0
*/
B allowPolicyLockout(boolean allowPolicyLockout);

/**
* Puts an arbitrary header with the specified {@code name} and String {@code value} to this builder.
*
Expand Down
Expand Up @@ -135,4 +135,9 @@ public DittoHeadersAssert hasIsResponseRequired(final boolean expected) {
"flag indicating if a response is required");
}

public DittoHeadersAssert hasAllowPolicyLockout(final boolean expected) {
return assertContains(Optional.of(actual.isAllowPolicyLockout()), expected,
"flag indicating if policy lockout is allowed");
}

}
Expand Up @@ -328,6 +328,20 @@ public void ensureResponseRequiredIsFalseWhenSet() {
.hasIsResponseRequired(false);
}

@Test
public void ensureAllowPolicyLockoutIsFalseWhenSet() {
final DittoHeaders dittoHeaders = DittoHeaders.newBuilder()
.allowPolicyLockout(true)
.build();
DittoBaseAssertions.assertThat(dittoHeaders).hasAllowPolicyLockout(true);
}

@Test
public void ensureAllowPolicyLockoutIsFalseWhenNotSet() {
final DittoHeaders dittoHeaders = DittoHeaders.newBuilder().build();
DittoBaseAssertions.assertThat(dittoHeaders).hasAllowPolicyLockout(false);
}

@Test
public void ensureResponseRequiredStaysFalseEvenWhenAcksAreRequested() {
final DittoHeaders dittoHeaders = DittoHeaders.newBuilder()
Expand Down
Expand Up @@ -111,6 +111,7 @@ public final class ImmutableDittoHeadersTest {
private static final MetadataHeaderKey KNOWN_METADATA_HEADER_KEY = MetadataHeaderKey.parse("/foo/bar");
private static final JsonValue KNOWN_METADATA_VALUE = JsonValue.of("knownMetadata");
private static final MetadataHeaders KNOWN_METADATA_HEADERS;
private static final boolean KNOWN_ALLOW_POLICY_LOCKOUT = true;

static {
KNOWN_METADATA_HEADERS = MetadataHeaders.newInstance();
Expand Down Expand Up @@ -159,6 +160,7 @@ public void settingAllKnownHeadersWorksAsExpected() {
.timeout(KNOWN_TIMEOUT)
.putHeader(DittoHeaderDefinition.CONNECTION_ID.getKey(), KNOWN_CONNECTION_ID)
.expectedResponseTypes(KNOWN_EXPECTED_RESPONSE_TYPES)
.allowPolicyLockout(KNOWN_ALLOW_POLICY_LOCKOUT)
.build();

assertThat(underTest).isEqualTo(expectedHeaderMap);
Expand Down Expand Up @@ -376,6 +378,7 @@ public void toJsonReturnsExpected() {
.set(DittoHeaderDefinition.EXPECTED_RESPONSE_TYPES.getKey(),
expectedResponseTypesToJsonArray(KNOWN_EXPECTED_RESPONSE_TYPES))
.set(DittoHeaderDefinition.PUT_METADATA.getKey(), KNOWN_METADATA_HEADERS.toJson())
.set(DittoHeaderDefinition.ALLOW_POLICY_LOCKOUT.getKey(), KNOWN_ALLOW_POLICY_LOCKOUT)
.build();
final Map<String, String> allKnownHeaders = createMapContainingAllKnownHeaders();

Expand Down Expand Up @@ -580,6 +583,7 @@ private static Map<String, String> createMapContainingAllKnownHeaders() {
result.put(DittoHeaderDefinition.EXPECTED_RESPONSE_TYPES.getKey(),
expectedResponseTypesToJsonArray(KNOWN_EXPECTED_RESPONSE_TYPES).toString());
result.put(DittoHeaderDefinition.PUT_METADATA.getKey(), KNOWN_METADATA_HEADERS.toJsonString());
result.put(DittoHeaderDefinition.ALLOW_POLICY_LOCKOUT.getKey(), String.valueOf(KNOWN_ALLOW_POLICY_LOCKOUT));

return result;
}
Expand Down
Expand Up @@ -55,6 +55,7 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
Expand All @@ -64,11 +65,10 @@
@RunWith(Parameterized.class)
public final class DittoProtocolAdapterParameterizedTest {

private static final org.slf4j.Logger LOGGER =
LoggerFactory.getLogger(DittoProtocolAdapterParameterizedTest.class);
private static final Logger LOGGER = LoggerFactory.getLogger(DittoProtocolAdapterParameterizedTest.class);
private static final DittoHeaders LIVE_DITTO_HEADERS = DittoHeaders.newBuilder().channel(LIVE.getName()).build();

// mock all adapaters and give them a name
// mock all adapters and give them a name
private static Adapter<ThingQueryCommand<?>> thingQueryCommandAdapter =
mock(Adapter.class, "ThingQueryCommandAdapter");
private static Adapter<ThingQueryCommandResponse<?>> thingQueryCommandResponseAdapter =
Expand Down
Expand Up @@ -95,9 +95,14 @@ public static <T extends PolicyCommand> Optional<T> authorizePolicyCommand(final
final ResourceKey policyResourceKey = PoliciesResourceType.policyResource(command.getResourcePath());
final AuthorizationContext authorizationContext = command.getDittoHeaders().getAuthorizationContext();
final boolean authorized;
if (command instanceof PolicyModifyCommand) {
final String permission = Permission.WRITE;
authorized = enforcer.hasUnrestrictedPermissions(policyResourceKey, authorizationContext, permission);
if (command instanceof CreatePolicy) {
if (command.getDittoHeaders().isAllowPolicyLockout()) {
authorized = true;
} else {
authorized = hasUnrestrictedWritePermission(enforcer, policyResourceKey, authorizationContext);
}
} else if (command instanceof PolicyModifyCommand) {
authorized = hasUnrestrictedWritePermission(enforcer, policyResourceKey, authorizationContext);
} else {
final String permission = Permission.READ;
authorized = enforcer.hasPartialPermissions(policyResourceKey, authorizationContext, permission);
Expand All @@ -108,6 +113,11 @@ public static <T extends PolicyCommand> Optional<T> authorizePolicyCommand(final
: Optional.empty();
}

private static boolean hasUnrestrictedWritePermission(final Enforcer enforcer, final ResourceKey policyResourceKey,
final AuthorizationContext authorizationContext) {
return enforcer.hasUnrestrictedPermissions(policyResourceKey, authorizationContext, Permission.WRITE);
}

/**
* Limit view on entity of {@code PolicyQueryCommandResponse} by enforcer.
*
Expand Down

0 comments on commit 3a288f6

Please sign in to comment.