Add client auth policies #246

chopshop1 · 2020-03-04T15:53:52Z

merge master

… scope and resource permission types (keycloak#220)

mrparkers · 2020-03-04T15:59:03Z

.circleci/config.yml

@@ -64,16 +64,20 @@ jobs:
    docker:
      - <<: *go_image
      - image: jboss/keycloak:7.0.1
+        command: ["-b", "0.0.0.0", "-Dkeycloak.profile.feature.upload_scripts=enabled"]


You can add this snippet to the keycloak_env anchor so it can be reused in the other test job

mrparkers · 2020-03-04T16:01:07Z

example/client_authorization_policys.tf

@@ -0,0 +1,174 @@
+resource keycloak_realm test {


I think most of these examples are going to conflict with the pre-existing main.tf file in the example folder. Could you rename the resources in this file so they don't conflict with main.tf? It's okay to hardcode any strings you want here, these files really just serve as an example of how to use the provider.

You are also free to reuse the resources that have already been created in main.tf, meaning that you can just leave them in that file and use variable interpolation to depend on them (like you are on line 12).

mrparkers · 2020-03-04T16:02:40Z

keycloak/keycloak_client.go

@@ -280,6 +281,7 @@ func (keycloakClient *KeycloakClient) sendRequest(request *http.Request) ([]byte
 	}

 	if response.StatusCode >= 400 {
+		fmt.Println(string(body), "Error with the response")


We're already printing the body on line 280, is there a reason this is needed?

In case you weren't aware, you can set the environment variable TF_LOG to DEBUG to enable debug logging which should print out a bunch of http request/response information.

mrparkers · 2020-03-04T16:05:22Z

provider/resource_keycloak_openid_client_authorization_aggregate_policy_test.go

+	resource.Test(t, resource.TestCase{
+		Providers: testAccProviders,
+		PreCheck:  func() { testAccPreCheck(t) },
+		// CheckDestroy: testResourceKeycloakOpenidClientAuthorizationAggregatePolicyDestroy(),


is there a reason this is commented out?

mrparkers · 2020-03-04T16:05:37Z

provider/resource_keycloak_openid_client_authorization_aggregate_policy_test.go

+	})
+}
+
+func testResourceKeycloakOpenidClientAuthorizationAggregatePolicy_basic(realm, clientId string) string {


nitpick: could you move this function to the bottom of this file?

mrparkers · 2020-03-04T16:23:45Z

provider/resource_keycloak_openid_client_authorization_group_policy_test.go

+	})
+}
+
+func testResourceKeycloakOpenidClientAuthorizationGroupPolicy_basic(realm, clientId string) string {


nitpick: can you move this to the bottom of the file?

mrparkers · 2020-03-04T16:24:01Z

provider/resource_keycloak_openid_client_authorization_js_policy_test.go

+	})
+}
+
+func testResourceKeycloakOpenidClientAuthorizationJSPolicy_basic(realm, clientId string) string {


nitpick: can you move this to the bottom of the file?

mrparkers · 2020-03-04T16:26:11Z

provider/resource_keycloak_openid_client_authorization_role_policy.go

+				Type:     schema.TypeString,
+				Optional: true,
+			},
+			"role": {


I can see in the GUI that this policy supports realm roles and client roles, which is this attribute supposed to represent? And it looks like I can add more than one role to the policy, is there a reason you specified MinItems to be 1?

I just checked the Chrome network inspector when creating a role policy, and I see that while bifurcated in the UI, the API call combines all the roles into one object:

Note the request above had one realm role and one client role.

I think the reason for min items equaling 1 is because a role policy essentially does nothing if there are no roles specified.

Thanks Ryan!
@mrparkers I can remove the MinItems if you'd like.

My bad, I misread MinItems to be MaxItems. Thanks for the clarification on the roles.

mrparkers · 2020-03-04T16:26:36Z

provider/resource_keycloak_openid_client_authorization_time_policy_test.go

+	})
+}
+
+func testResourceKeycloakOpenidClientAuthorizationTimePolicy_basic(realm, policyName, clientId string) string {


nitpick: can you move this to the bottom of the file?

mrparkers · 2020-03-04T16:26:49Z

provider/resource_keycloak_openid_client_authorization_user_policy_test.go

+	})
+}
+
+func testResourceKeycloakOpenidClientAuthorizationUserPolicy_basic(realm, clientId string) string {


nitpick: can you move this to the bottom of the file?

…rization_user_policy_test to fix tests

chopshop1 · 2020-03-04T19:48:44Z

@mrparkers I made the requested changes! Could you please take another look.

mrparkers · 2020-03-05T15:41:15Z

The changes you made so far look good, although there are still 5 other comments I left that I didn't see changes for.

I think GitHub collapses comments when there are too many, I had to click "Load More" on the part of the page that says "5 hidden conversations"

…removed unessary schema keys

chopshop1 · 2020-03-06T02:42:23Z

Woops! Just made the last 5 changes. Let me know what you think.
@mrparkers

mrparkers · 2020-03-09T14:56:05Z

provider/validation_helpers.go

+	"fmt"
+)
+
+func logicKeyValidation(val interface{}, key string) (warns []string, errs []error) {


Thanks for adding this! Although I probably should have let you know about the validation package which gives you the StringInSlice function that could have made this implementation easier. Take a look at this example: /~https://github.com/mrparkers/terraform-provider-keycloak/blob/f4770b0a3726a82d0fd0176cba328b4b2dda1389/provider/resource_keycloak_ldap_user_federation.go#L65

Could you change this to use this function?

Oh cool! Just replaced that with the validation package. 👍

mrparkers · 2020-03-09T23:20:44Z

I left another comment that might be hard to see since it's in the collapsed conversation: #246 (comment)

mrparkers

Thanks for making the changes I suggested, LGTM

Aria and others added 18 commits March 4, 2020 10:32

Merge pull request #2 from mrparkers/master

ca47460

merge master

Add all the client authz policy resources

4b5523b

fixed authz policy resources errors

6bf16df

added tests for role policy

af745c0

added tests for time policy

cd50876

added tests for authorization client policy

c6f4146

added tests for authorization user policy

51dfc6f

added tests for authorization group policy

22486ad

added tests for authorization aggregate policy

b705cd5

fixed user tests, and removed print statments

66df902

added test for authorization js policy

3a4b3f3

enabled feature upload_scripts

f646df4

disabled feature upload_scripts

38cc6e3

update keycloak_openid_client_authorization_permission to handle both…

87bf4e7

… scope and resource permission types (keycloak#220)

removed printlogs

98c6a5a

debugging js policy failure

2e3caab

print body when we get an error

1819943

added examples for client authorization policys

5bdf948

chopshop1 mentioned this pull request Mar 4, 2020

Add client authz policies #221

Closed

mrparkers suggested changes Mar 4, 2020

View reviewed changes

mrparkers mentioned this pull request Mar 4, 2020

Javascript or Drools policies? #245

Closed

Aria added 4 commits March 4, 2020 14:20

fixes for mrparkers comments

f7673b4

attempting to fix pipeline

30bd291

attempting to fix pipeline

d316582

added new line to the bottom of resource_keycloak_openid_client_autho…

715fc06

…rization_user_policy_test to fix tests

Aria added 3 commits March 5, 2020 20:55

added in logic validation helper, added generic resource policy, and …

56515e6

…removed unessary schema keys

fixed logicKeyValidation function

8b21047

fixed logicKeyValidation function

85e6edc

removed unused policy import functions

abbe5b1

mrparkers reviewed Mar 9, 2020

View reviewed changes

replaced custom validation function with terraform one

4232ddf

removed unused attributes

812ead6

mrparkers approved these changes Mar 10, 2020

View reviewed changes

mrparkers merged commit c78a9c3 into keycloak:master Mar 10, 2020

sashokbg mentioned this pull request Dec 29, 2022

Client Authorization Type Not Found #787

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add client auth policies #246

Add client auth policies #246

chopshop1 commented Mar 4, 2020

mrparkers Mar 4, 2020

mrparkers Mar 4, 2020

mrparkers Mar 4, 2020

mrparkers Mar 4, 2020

mrparkers Mar 4, 2020

mrparkers Mar 4, 2020

mrparkers Mar 4, 2020

mrparkers Mar 4, 2020

mrparkers Mar 4, 2020

RyanAtWare2Go Mar 4, 2020

RyanAtWare2Go Mar 4, 2020

chopshop1 Mar 4, 2020

mrparkers Mar 5, 2020

mrparkers Mar 4, 2020

mrparkers Mar 4, 2020

chopshop1 commented Mar 4, 2020

mrparkers commented Mar 5, 2020

chopshop1 commented Mar 6, 2020

mrparkers Mar 9, 2020

chopshop1 Mar 9, 2020

mrparkers commented Mar 9, 2020

mrparkers left a comment

Add client auth policies #246

Add client auth policies #246

Conversation

chopshop1 commented Mar 4, 2020

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chopshop1 commented Mar 4, 2020

mrparkers commented Mar 5, 2020

chopshop1 commented Mar 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrparkers commented Mar 9, 2020

mrparkers left a comment

Choose a reason for hiding this comment