Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

fix(keycloak-authz): make relationship field auth configurable at GraphQL field level #2101

Merged
merged 1 commit into from
Sep 22, 2020

Conversation

craicoverflow
Copy link

@craicoverflow craicoverflow commented Sep 22, 2020

Relationship fields needed to be configured with the database field name. This made it confusing to set auth rules since often the user does not know what the database column name is (it usually maps to {fieldName}Id, but this knowledge is not apparent from the schema and is hard to track.

This fixes that so it can be configured with the model field name instead.

This PR also updates the docs, which showed configuration happening from the 1:M side, whereas it should be from the M:1 side.

Before and after:

Comment: {
  relations: {
-    noteId: {
+    note: {
       roles: ['admin']
    }
  }
}

Copy link
Contributor

@machi1990 machi1990 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me.

Can we add a test?

@craicoverflow
Copy link
Author

craicoverflow commented Sep 22, 2020

A test exists already at https://github.com/craicoverflow/graphback/blob/keycloak-fix/packages/graphback-keycloak-authz/tests/KeycloakCrudService.test.ts#L358 .. but I think I can improve this as it can be configured to give a false positive (as it was doing already). I will work on that now.

@machi1990
Copy link
Contributor

A test exists already at https://github.com/craicoverflow/graphback/blob/keycloak-fix/packages/graphback-keycloak-authz/tests/KeycloakCrudService.test.ts#L358 .. but I think I can improve this as it can be configured to give a false positive (as it was doing already). I will work on that now.

Thanks

@craicoverflow
Copy link
Author

A test exists already at https://github.com/craicoverflow/graphback/blob/keycloak-fix/packages/graphback-keycloak-authz/tests/KeycloakCrudService.test.ts#L358 .. but I think I can improve this as it can be configured to give a false positive (as it was doing already). I will work on that now.

Actually...this test validates is correctly, no need for changes.

@craicoverflow craicoverflow merged commit 6e513c0 into aerogear:master Sep 22, 2020
@craicoverflow craicoverflow deleted the keycloak-fix branch September 22, 2020 14:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants