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

change type of enum PolicyEnforcementMode from int to string #315

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

yue-wen
Copy link

@yue-wen yue-wen commented Oct 29, 2021

Get error to convert json to *gocloak.Client{}

[
  {
    "clientId": "test",
    "name": "test",
    "authorizationSettings": {
      "allowRemoteResourceManagement": true,
      "policyEnforcementMode": "ENFORCING",
      "decisionStrategy": "UNANIMOUS"
    }
  }
]
clients := &[]*gocloak.Client{}

err := json.Unmarshal(clientsJSON, clients)
 json: cannot unmarshal string into Go struct field ResourceServerRepresentation.authorizationSettings.policyEnforcementMode of type gocloak.PolicyEnforcementMode

the type of enum PolicyEnforcementMode must change to string

It should same as enum Logic and DecisionStrategy

then it will same style as Logic and DecisionStrategy
@codecov
Copy link

codecov bot commented Oct 29, 2021

Codecov Report

Merging #315 (c99afa5) into main (7401dd2) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #315      +/-   ##
==========================================
+ Coverage   76.29%   76.30%   +0.01%     
==========================================
  Files           4        4              
  Lines        1911     1912       +1     
==========================================
+ Hits         1458     1459       +1     
  Misses        314      314              
  Partials      139      139              
Impacted Files Coverage Δ
models.go 92.80% <ø> (ø)
utils.go 96.42% <100.00%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7401dd2...c99afa5. Read the comment docs.

@Nerzal
Copy link
Owner

Nerzal commented Oct 29, 2021

Hi do you know if this is potentially breaking the API for older keycloak versions?
If that is the case, we'd introduce a new major version after merging, and state that change in the changelog :)

@yue-wen
Copy link
Author

yue-wen commented Oct 29, 2021

@Nerzal You are right. I don't know if it brings breaking change. We should make a new major version.

In current state, i didn't see there difference between enum PolicyEnforcementMode, Logic and DecisionStrategy.

// PolicyEnforcementMode is an enum type for PolicyEnforcementMode of ResourceServerRepresentation
type PolicyEnforcementMode int

// PolicyEnforcementMode values
const (
	ENFORCING PolicyEnforcementMode = iota
	PERMISSIVE
	DISABLED
)

// Logic is an enum type for policy logic
type Logic string
// Logic values
var (
	POSITIVE = LogicP("POSITIVE")
	NEGATIVE = LogicP("NEGATIVE")
)
// DecisionStrategy is an enum type for DecisionStrategy of PolicyRepresentation
type DecisionStrategy string
// DecisionStrategy values
var (
	AFFIRMATIVE = DecisionStrategyP("AFFIRMATIVE")
	UNANIMOUS   = DecisionStrategyP("UNANIMOUS")
	CONSENSUS   = DecisionStrategyP("CONSENSUS")
)

keycloak docs:

https://www.keycloak.org/docs-api/8.0/rest-api/index.html#_resourceserverrepresentation

Name Schema
allowRemoteResourceManagementoptional boolean
clientIdoptional string
decisionStrategyoptional enum (AFFIRMATIVE, UNANIMOUS, CONSENSUS)
idoptional string
nameoptional string
policiesoptional < PolicyRepresentation > array
policyEnforcementModeoptional enum (ENFORCING, PERMISSIVE, DISABLED)
resourcesoptional < ResourceRepresentation > array
scopesoptional < ScopeRepresentation > array

@Nerzal Nerzal merged commit 5cd04ca into Nerzal:main Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants