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

feat: wildcard type restriction #95

Merged
merged 3 commits into from
Nov 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
],
"author": "OpenFGA",
"dependencies": {
"@openfga/sdk": "^0.1.0",
"@openfga/sdk": "^0.1.1",
"nearley": "^2.20.1"
},
"devDependencies": {
Expand Down
3 changes: 3 additions & 0 deletions src/api-to-friendly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ const apiToFriendlyRelation = (
if (relationReference.relation) {
return `${relationReference.type}#${relationReference.relation}`;
}
if (relationReference.wildcard) {
return `${relationReference.type}:*`;
}
return relationReference.type;
}) || [];

Expand Down
40 changes: 32 additions & 8 deletions src/check-dsl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,19 @@ const defaultError = (lines: string[]) => {
};
};

interface destructedAssignableType {
decodedType: string;
decodedRelation?: string;
isWildcard: boolean;
}

// helper function to figure out whether the specified allowable types
// are tuple to user set. If so, return the type and relationship.
// Otherwise, return null as relationship
function destructTupleToUserset(allowableType: string): string[] {
return allowableType.split("#", 2);
function destructTupleToUserset(allowableType: string): destructedAssignableType {
const isWildcard = allowableType.includes(":*");
rhamzeh marked this conversation as resolved.
Show resolved Hide resolved
const splittedWords = allowableType.replace(":*", "").split("#");
return { decodedType: splittedWords[0], decodedRelation: splittedWords[1], isWildcard };
}

// helper function to parse thru a child relation to see if there are unique entry points.
Expand Down Expand Up @@ -92,7 +100,7 @@ function childHasEntryPoint(
if (childDef.rewrite === RewriteType.Direct) {
// we can safely assume that direct rewrite (i.e., it is a self/this), there are direct entry point
for (const item of allowedTypes) {
const [decodedType, decodedRelation] = destructTupleToUserset(item);
const { decodedType, decodedRelation } = destructTupleToUserset(item);
if (!decodedRelation) {
// this is not a tuple set and is a straight type, we can return true right away
return true;
Expand All @@ -114,7 +122,7 @@ function childHasEntryPoint(
// to see if there are unique entry point
const fromPossibleTypes = transformedTypes[type].relations[childDef.from].allowedTypes;
for (const fromType of fromPossibleTypes) {
const [decodedType] = destructTupleToUserset(fromType);
const { decodedType } = destructTupleToUserset(fromType);

// For now, we just look at the type without seeing whether the user set
// of the type is reachable too in the case of tuple to user set.
Expand Down Expand Up @@ -226,8 +234,16 @@ function childDefDefined(
reporter.assignableRelationMustHaveTypes({ lineIndex, value: relation });
}
for (const item of fromPossibleTypes) {
const [decodedType, decodedRelation] = destructTupleToUserset(item);
if (decodedRelation) {
const { decodedType, decodedRelation, isWildcard } = destructTupleToUserset(item);
if (isWildcard && decodedRelation) {
// we cannot have both wild carded and relation at the same time
const typeIndex = getTypeLineNumber(type, lines);
const lineIndex = getRelationLineNumber(relation, lines, typeIndex);
reporter.assignableTypeWildcardRelation({
lineIndex,
value: item,
});
} else if (decodedRelation) {
if (!transformedTypes[decodedType] || !transformedTypes[decodedType].relations[decodedRelation]) {
// type/relation is not defined
const typeIndex = getTypeLineNumber(type, lines);
Expand Down Expand Up @@ -282,8 +298,16 @@ function childDefDefined(
const [fromTypes, isValid] = allowableTypes(transformedTypes, type, childDef.from);
if (isValid) {
for (const item of fromTypes) {
const [decodedType, decodedRelation] = destructTupleToUserset(item);
if (decodedRelation) {
const { decodedType, decodedRelation, isWildcard } = destructTupleToUserset(item);
if (isWildcard && decodedRelation) {
// we cannot have both wild carded and relation at the same time
const typeIndex = getTypeLineNumber(type, lines);
const lineIndex = getRelationLineNumber(relation, lines, typeIndex);
reporter.assignableTypeWildcardRelation({
lineIndex,
value: item,
});
} else if (decodedRelation) {
const typeIndex = getTypeLineNumber(type, lines);
const lineIndex = getRelationLineNumber(relation, lines, typeIndex);
reporter.tupleUsersetRequiresDirect({
Expand Down
5 changes: 5 additions & 0 deletions src/friendly-to-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,18 @@ export const friendlySyntaxToApiSyntax = (

allowedTypes?.forEach((allowedType: string) => {
metadataAvailable = true;
const isWildcardRestricted = allowedType.includes(":*");
allowedType = allowedType.replace(":*", "");
const [userType, usersetRelation] = allowedType.split("#");
const toAdd: any = {
type: userType,
};
if (usersetRelation) {
toAdd["relation"] = usersetRelation;
}
if (isWildcardRestricted) {
toAdd["wildcard"] = {};
}
relationsMetadataMap[relationName]["directly_related_user_types"].push(toAdd);
});
});
Expand Down
18 changes: 14 additions & 4 deletions src/openfga.ne
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,18 @@ _word -> ([a-z] | [A-Z] | [0-9] | "_" | "-" | "," | "&" | "+" | "/"

_colon -> _optional_space ":"

_relation_types -> "[" _array_of_types "]" {%
data => ({allowedTypes: data[1]})
_relation_types -> "[" _optional_space (_array_of_types):? "]" {%
data => (data[2])? {allowedTypes: data[2][0]} : {allowedTypes: []}
%}

_array_of_types -> ("$"):? ([a-zA-Z0-9_#\-,\s]):* {%
data => data.flat(3).join('').split(",").map(i => i.trim()).filter(word => word.length)
_array_of_types -> (_allowed_naming _optional_space _comma _optional_space):* _allowed_naming _optional_space {%
data => {
if (data[0].length) {
// @ts-ignore
return [...data[0].map((datum) => datum[0]), data[1]];
}
return [data[1]];
}
%}

_from -> "from"
Expand All @@ -230,12 +236,16 @@ _no_relations -> "none" (_newline):*
_naming -> (("$"):? ( [a-z] | [A-Z] | [0-9] | "_" | "-" ):+) {%
data => data.flat(3).join('').trim()
%}
_allowed_naming -> (("$"):? ( [a-z] | [A-Z] | [0-9] | "_" | "-" | "#" ):+ (":*"):?) {%
rhamzeh marked this conversation as resolved.
Show resolved Hide resolved
data => data.flat(3).join('').trim()
%}
_optional_space -> " ":*
_spacing -> " ":+
_newline -> _optional_space "\n"
_model -> "model"
_schema -> " schema"
_period -> "."
_comma -> ","
_version -> (([0-9]):+) _period (([0-9]):+)
_version_10 -> "1.0"
_version_11 -> "1.1"
14 changes: 14 additions & 0 deletions src/reporters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,17 @@ export const reportAssignableRelationMustHaveTypes = ({ markers, lines, lineInde
});
};

export const reportAssignableTypeWildcardRelation = ({ markers, lines, lineIndex, value }: ReporterOpts) => {
reportError({
message: `Type restriction '${value}' cannot contain both wildcard and relation`,
markers,
lineIndex,
lines,
value,
relatedInformation: { type: "type-wildcard-relation" },
});
};

export const reportInvalidButNot = ({ markers, lines, lineIndex, value, clause }: ReporterOpts) => {
reportError({
message: `Cannot self-reference (\`${value}\`) within \`${Keywords.BUT_NOT}\` clause.`,
Expand Down Expand Up @@ -370,6 +381,9 @@ export const report = function ({ markers, lines }: Pick<BaseReporterOpts, "mark
assignableRelationMustHaveTypes: ({ lineIndex, value }: Omit<ReporterOpts, "markers" | "lines">) =>
reportAssignableRelationMustHaveTypes({ lineIndex, markers, lines, value }),

assignableTypeWildcardRelation: ({ lineIndex, value }: Omit<ReporterOpts, "markers" | "lines">) =>
reportAssignableTypeWildcardRelation({ lineIndex, markers, lines, value }),

invalidRelation: ({ lineIndex, value, validRelations }: Omit<ReporterOpts, "markers" | "lines">) =>
reportInvalidRelation({
lineIndex,
Expand Down
86 changes: 86 additions & 0 deletions tests/data/model-validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,33 @@ type org
},
],
},
{
name: "type restriction cannot contains both wildcard and relation",
friendly: `model
schema 1.1
type user
type department
relations
define member: [user]
type org
relations
define reader: [department, department#member:*]
`,
expectedError: [
{
endColumn: 52,
endLineNumber: 9,
message: "Type restriction 'department#member:*' cannot contain both wildcard and relation",
relatedInformation: {
type: "type-wildcard-relation",
},
severity: 8,
source: "linter",
startColumn: 33,
startLineNumber: 9,
},
],
},
{
name: "unsupported schema version should yield error",
friendly: `model
Expand Down Expand Up @@ -940,6 +967,27 @@ type group
},
],
},
{
name: "incorrect wildcard restriction should be raised",
friendly: `model
schema 1.1
type user
type group
relations
define member: [user, user:*:*]
`,
expectedError: [
{
endColumn: 35,
endLineNumber: 6,
message: "Invalid syntax",
adriantam marked this conversation as resolved.
Show resolved Hide resolved
severity: 8,
source: "linter",
startColumn: 33,
startLineNumber: 6,
},
],
},
// The following are valid cases and should not result in error
{
name: "simple model 1.0",
Expand Down Expand Up @@ -1104,6 +1152,44 @@ type document
define parent: [folder]
define viewer: [user] or viewer from parent
type user
`,
expectedError: [],
},
{
name: "model 1.1 wildcard restricted type",
friendly: `model
schema 1.1
type folder
relations
define viewer: [user, user:*]

type user
`,
expectedError: [],
},
{
name: "model 1.1 wildcard restricted type in the middle",
friendly: `model
schema 1.1
type folder
relations
define viewer: [user, user:*, group]

type user
type group
`,
expectedError: [],
},
{
name: "model 1.1 with spacing in allowed type",
friendly: `model
schema 1.1
type folder
relations
define viewer: [ user , user:* , group ]

type user
type group
`,
expectedError: [],
},
Expand Down
29 changes: 29 additions & 0 deletions tests/data/test-models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,35 @@ type document
type document
relations
define viewer: [user,group]
`,
},
{
name: "wildcard restriction conversion",
json: {
schema_version: "1.1",
type_definitions: [
{
type: "document",
relations: {
viewer: {
this: {},
},
},
metadata: {
relations: {
viewer: {
directly_related_user_types: [{ type: "user" }, { type: "user", wildcard: {} }, { type: "group" }],
},
},
},
},
],
},
friendly: `model
schema 1.1
type document
relations
define viewer: [user,user:*,group]
`,
},
];
13 changes: 13 additions & 0 deletions tests/dsl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,19 @@ type app
expect(result.length).toEqual(1);
});

it("should only return single result for wildcard restricted type", () => {
const result = innerParseDSL(`model
schema 1.1
type user
type employee
type team
relations
define member: [ user , user:*, employee ]

`);
expect(result.length).toEqual(1);
});

it("should only return single result for complex 1.1 model with spaces", () => {
const result = innerParseDSL(`model
schema 1.1
Expand Down