add draft OpenFGA client #10504

leovalais · 2025-01-23T12:15:36Z

Note

This is a lengthy PR which also requires some amount of OpenFGA knowledge to be understood. It's probably better to peer review it.

Draft OpenFGA client.

Will be moved into editoast's workspace when ready (it's currently outside for dev comfort)
Only a few operations have been implemented
Meant to remain a new sub-crate which will be included in editoast_authz (in which we'll map the authorization model, define our objects, their conversion, relations, etc.)
The fga binary is necessary to run unit tests for now: https://github.com/openfga/cli

TODO:

About macros

This crates introduces a few macro_rules, which are only used for tests currently.

relations! allows defining typed relations concisely to allow quick comparisons with the OpenFGA schema. I think we should keep this one, that'll help the review process and reduce straightforward boilerplate.
fga_type! declares the newtype structs representing the types of OpenFGA and generates a few implementations. I think this one should remain a #[cfg(test)] definition. However I think derive macros derive(fga::User, fga::Object) would help us and cost nothing as they are trivial to implement.
fga! allows manipulating and instantiating types and relations with the same syntax as OpenFGA, but in a type-safe way. This is particularly useful for the conciseness of unit tests. I think it should be exported.

codecov-commenter · 2025-01-23T12:17:46Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 271 lines in your changes missing coverage. Please review.

Project coverage is 82.20%. Comparing base (34159e3) to head (e5416a8).
Report is 19 commits behind head on dev.

Files with missing lines	Patch %	Lines
fga/src/client.rs	0.00%	271 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev   #10504      +/-   ##
==========================================
- Coverage   82.56%   82.20%   -0.37%     
==========================================
  Files        1084     1085       +1     
  Lines      107257   107811     +554     
  Branches      729      729              
==========================================
+ Hits        88559    88622      +63     
- Misses      18656    19147     +491     
  Partials       42       42

Flag	Coverage Δ
editoast	`74.39% <ø> (-0.01%)`	⬇️
front	`90.51% <ø> (+0.04%)`	⬆️
gateway	`2.18% <ø> (ø)`
osrdyne	`2.98% <0.00%> (-0.31%)`	⬇️
railjson_generator	`87.58% <ø> (ø)`
tests	`87.91% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

woshilapin

Only a first pass on src/client.rs. I'll take a look at the rest later.

fga/Cargo.lock

fga/src/client.rs

fga/Cargo.toml

flomonster

Small review of the docker-compose

docker-compose.yml

woshilapin · 2025-02-12T15:14:55Z

fga/src/model.rs

+        format!("{}:{}", Self::NAMESPACE, self.id())
+    }
+
+    fn tbpa() -> Tbpa<Self> {


Maybe need to be renamed all? Or wildcard?

wildcard, as discussed

fga/Cargo.toml

woshilapin · 2025-02-12T15:19:50Z

fga/Cargo.toml

We're missing an additionnal step in the CI to launch the tests? (or do we wait for this crate to be merged into editoast's workspace?

fga/src/model.rs

woshilapin · 2025-02-13T08:36:45Z

fga/src/doctest_setup.rs

+
+use fga::model::Relation as _;


Can be move at the top.

woshilapin · 2025-02-13T08:38:16Z

fga/src/model.rs

+    const NAMESPACE: &'static str;
+
+    fn id(&self) -> &str;
+
+    fn fga_ident(&self) -> String {
+        format!("{}:{}", Self::NAMESPACE, self.id())
+    }


This piece of code is the same between User and Object. How about making an trait Id that both Object and User would be a subset of?

done, by yourself

woshilapin · 2025-02-13T08:42:07Z

fga/src/model.rs

+pub trait AsUser {
+    type User: User;
+
+    fn id(&self) -> &str;
+
+    fn fga_ident(&self) -> String {
+        let id = self.id();
+        if id == "*" {
+            panic!(
+                "Refusing to generate an identifier for a type-bound public access\n\
+                without using the `fga::model::Tpba` type.\n\
+                This is a security measure to avoid having user identifiers coerced to\n\
+                public accesses maliciously."
+            );
+        }
+        format!("{}:{}", Self::User::NAMESPACE, id)
+    }
+}


I don't understand why we need this trait. It's exactly the same than User (except User has a bit more). It feels that everywhere a AsUser would be expected, a User bound would be enough.

Signed-off-by: Leo Valais <[email protected]>

Signed-off-by: Jean SIMARD <[email protected]>

Signed-off-by: Leo Valais <[email protected]>

Signed-off-by: Jean SIMARD <[email protected]>

Signed-off-by: Leo Valais <[email protected]>

Co-authored-by: Florian Amsallem <[email protected]> Signed-off-by: Léo Valais <[email protected]>

Signed-off-by: Leo Valais <[email protected]>

Co-authored-by: Jean SIMARD <[email protected]> Signed-off-by: Léo Valais <[email protected]>

Signed-off-by: Florian Amsallem <[email protected]>

Signed-off-by: Leo Valais <[email protected]>

leovalais mentioned this pull request Jan 23, 2025

Decrease the amount of roles and use OpenFGA as backend #10503

Draft

6 tasks

leovalais force-pushed the lva/openfga-client branch 7 times, most recently from 131388a to ac0adfd Compare February 6, 2025 12:27

leovalais requested review from woshilapin, flomonster and Tguisnet February 6, 2025 15:28

leovalais self-assigned this Feb 6, 2025

woshilapin reviewed Feb 11, 2025

View reviewed changes

leovalais force-pushed the lva/openfga-client branch from a767ba2 to 4ee9be3 Compare February 12, 2025 12:30

leovalais commented Feb 12, 2025

View reviewed changes

fga/src/client.rs Outdated Show resolved Hide resolved

woshilapin reviewed Feb 12, 2025

View reviewed changes

fga/Cargo.toml Outdated Show resolved Hide resolved

github-actions bot added the area:editoast Work on Editoast Service label Feb 17, 2025

flomonster reviewed Feb 18, 2025

View reviewed changes

github-actions bot removed the area:editoast Work on Editoast Service label Feb 18, 2025

woshilapin reviewed Feb 18, 2025

View reviewed changes

leovalais added 9 commits February 18, 2025 16:22

here we go again

4f911ef

Signed-off-by: Leo Valais <[email protected]>

add draft OpenFGA client

956116d

Signed-off-by: Leo Valais <[email protected]>

remove useless impl Serialize

9dffd53

Signed-off-by: Leo Valais <[email protected]>

add params contextual_tuples and authorization_model_id

20c2164

Signed-off-by: Leo Valais <[email protected]>

fix test api

cde7909

Signed-off-by: Leo Valais <[email protected]>

page_size

fa99a79

Signed-off-by: Leo Valais <[email protected]>

bye

c76a275

Signed-off-by: Leo Valais <[email protected]>

support authorization model IDs API parameters

15c1f17

Signed-off-by: Leo Valais <[email protected]>

rewrite get_stores using ContinuationUnfolder

8f5faba

Signed-off-by: Leo Valais <[email protected]>

leovalais and others added 21 commits February 18, 2025 16:22

example fixed

ad8d633

Signed-off-by: Leo Valais <[email protected]>

update doc

328fe9d

Signed-off-by: Leo Valais <[email protected]>

panic on public access wildcard abuse

dafef37

Signed-off-by: Leo Valais <[email protected]>

expose and document compile_model

bf01b80

Signed-off-by: Leo Valais <[email protected]>

rm outdated warning

8edc920

Signed-off-by: Leo Valais <[email protected]>

typos and missing links

d182989

Signed-off-by: Leo Valais <[email protected]>

editoast: simplify the continuation state for streams

6099d44

Signed-off-by: Jean SIMARD <[email protected]>

in_current_span

b640d87

Signed-off-by: Leo Valais <[email protected]>

fga: factorized the Id trait for User and Object

921ad43

Signed-off-by: Jean SIMARD <[email protected]>

fga: remove context from ContinuationState

8f9187a

Signed-off-by: Jean SIMARD <[email protected]>

update model repr

250488e

fix tbpa fga_ident

28372f9

add list_users

a5ef8b1

fix doctests

d7ef4ba

Signed-off-by: Leo Valais <[email protected]>

list_usersets

eb3d9b8

Signed-off-by: Leo Valais <[email protected]>

use fga! everywhere in tests

4cb9d08

Signed-off-by: Leo Valais <[email protected]>

docker-compose + down migration

80a4318

Signed-off-by: Leo Valais <[email protected]>

Update docker-compose.yml

316873f

Co-authored-by: Florian Amsallem <[email protected]> Signed-off-by: Léo Valais <[email protected]>

doc

efacd99

Signed-off-by: Leo Valais <[email protected]>

bye bye migration

5765f13

Signed-off-by: Leo Valais <[email protected]>

reset_store

5c43f4f

Signed-off-by: Leo Valais <[email protected]>

leovalais force-pushed the lva/openfga-client branch from d95db11 to 9da4085 Compare February 18, 2025 15:22

leovalais changed the base branch from lva/roles-openfga-edition to dev February 18, 2025 15:23

leovalais and others added 3 commits February 18, 2025 17:31

Update fga/src/model.rs

90e8086

Co-authored-by: Jean SIMARD <[email protected]> Signed-off-by: Léo Valais <[email protected]>

docker: stop exposing ports of editoast when run in pr-tests mode

eb08728

Signed-off-by: Florian Amsallem <[email protected]>

docker: add openfga config to all docker compose files

73606ae

Signed-off-by: Florian Amsallem <[email protected]>

flomonster force-pushed the lva/openfga-client branch from 6ea7ac1 to 73606ae Compare February 18, 2025 16:31

leovalais added 3 commits February 18, 2025 23:03

tip top

7d6841c

Signed-off-by: Leo Valais <[email protected]>

wildcard is the way

4279493

Signed-off-by: Leo Valais <[email protected]>

no id required here

e5416a8

Signed-off-by: Leo Valais <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add draft OpenFGA client #10504

add draft OpenFGA client #10504

leovalais commented Jan 23, 2025 •

edited

Loading

codecov-commenter commented Jan 23, 2025 •

edited

Loading

woshilapin left a comment

flomonster left a comment

woshilapin Feb 12, 2025

leovalais Feb 18, 2025

leovalais Feb 18, 2025

woshilapin Feb 12, 2025

woshilapin Feb 13, 2025

leovalais Feb 18, 2025

woshilapin Feb 13, 2025

leovalais Feb 18, 2025

leovalais Feb 18, 2025

leovalais Feb 18, 2025

woshilapin Feb 13, 2025

add draft OpenFGA client #10504

Are you sure you want to change the base?

add draft OpenFGA client #10504

Conversation

leovalais commented Jan 23, 2025 • edited Loading

About macros

codecov-commenter commented Jan 23, 2025 • edited Loading

Codecov Report

woshilapin left a comment

Choose a reason for hiding this comment

flomonster left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leovalais commented Jan 23, 2025 •

edited

Loading

codecov-commenter commented Jan 23, 2025 •

edited

Loading