openfga / language

Grammar for the OpenFGA modeling language
https://openfga.dev
Apache License 2.0
18 stars 7 forks source link

Incorrect line for type when looking up the typeIndex, when type is a substring #255

Closed d-jeffery closed 5 months ago

d-jeffery commented 6 months ago

Problem: incorrect type index return for asset when asset-category is given precedence in declaration.

Example:

image image

Fix: update each method to do a full string match, not a prefix match.

https://github.com/openfga/language/blob/7d0da9bc9c63f3885c512795279f0f0a025452b1/pkg/js/util/line-numbers.ts#L23

https://github.com/openfga/language/blob/7d0da9bc9c63f3885c512795279f0f0a025452b1/pkg/go/utils/line-numbers.go#L16

https://github.com/openfga/language/blob/7d0da9bc9c63f3885c512795279f0f0a025452b1/pkg/java/src/main/java/dev/openfga/language/validation/Dsl.java#L50

d-jeffery commented 6 months ago

This would also be present if the same logic is used for relations and conditions, where a prefix match is done.

Edit:

Easily replicated with model:

model
  schema 1.1
type user
type role
  relations
    define assigne: [user]
type asset-category
  relations
    define a1: [role#assignee]
type asset
  relations
    define a1: [role#assignee]

Fixed for validation by converting the startsWith to a match call with a more specific regex;

const getTypeLineNumber = (typeName: string, lines?: string[], skipIndex?: number) => {
  ...
  return lines.slice(skipIndex).findIndex((line: string) => line.trim().match(`type ${typeName}$`)) + skipIndex;
};

models-to-modules code here doesn't take an offendingRelation like the error generation during validation of regular models.