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

fix(config-api): issues in the API and attributes #2227

Closed
moabu opened this issue Aug 26, 2022 · 12 comments
Closed

fix(config-api): issues in the API and attributes #2227

moabu opened this issue Aug 26, 2022 · 12 comments
Assignees
Labels
kind-bug Issue or PR is a bug in existing functionality
Milestone

Comments

@moabu
Copy link
Member

moabu commented Aug 26, 2022

Documentation

  • Examples for patch requests are incorrect, as they lack the leading \ in the path. If the slash is not provided, the server responds with a 500 (which should probably be fixed to be a 400).

API discrepancies

JansFido2DynConfiguration

  • API missing attribute MetricReporterEnabled

ADDRESSED
Reported issue fixed in Spec

Attributes

  • API missing attribute Selected
  • API missing attribute Custom
  • API missing attribute Required (spelled 'requred' in the response object)
  • API missing attribute AdminCanAccess
  • API missing attribute AdminCanView
  • API missing attribute AdminCanEdit
  • API missing attribute UserCanAccess
  • API missing attribute UserCanView
  • API missing attribute UserCanEdit
  • API missing attribute WhitePagesCanView
  • API missing attribute BaseDn
  • Response is missing attribute nameIdType
  • Response is missing attribute jansHideOnDiscovery
  • Response is missing attribute attributeValidation
  • Response is missing attribute salt
  • Response is missing attribute scimCustomAttr
  • Response is missing attribute seeAlso
  • Response is missing attribute sourceAttribute
  • Response is missing attribute tooltip
  • Response is missing attribute usageType

ADDRESSED
Reported issue fixed in Spec
Response is missing attribute - if values is null then that attribute is not part of response.
Few attributes were redundant hence removed from spec

CouchbaseConfig

  • API missing attribute KVTimeout
  • API missing attribute QueryTimeout

ADDRESSED
Reported issue fixed in Spec

Custom scripts

  • API missing attribute LocationType
  • API missing attribute BaseDN

ADDRESSED
Reported issue fixed in Spec

AuthConfig

  • Type mismatch for authorizationRequestCustomAllowedParameters, according to API it should be array of strings, but server returns array of `CustomAllowedParameter

ADDRESSED
authorizationRequestCustomAllowedParameters returns array of AuthorizationRequestCustomParameter and spec is correct
image

NativePersistenceConfiguration

  • API missing attribute DisableAttemptUpdateBeforeInsert

ADDRESSED
Reported issue fixed in Spec

JWKs

  • DELETE on /jans-config-api/api/v1/config/jwks/{kid}: expects a scope https://jans.io/oauth/config/jwks.write, but instead it should be https://jans.io/oauth/config/jwks.delete, if it's to be consistent with other DELETE operations.

ADDRESSED
Modified DELETE on /jans-config-api/api/v1/config/jwks/{kid} to use https://jans.io/oauth/config/jwks.delete scope

UMAResource

  • Rev attribute is required, but not marked as such in the API. Not providing it results in a 500 error (it should be a 400). Also it should probably be of type int, as rev attributes in other entities are of type int and it seems to be parsed to an int by the server anyway.

ADDRESSED
by @yuriyz in 2e02d5e

ClientAttributes

  • runIntrospectionScriptBeforeAccessTokenAsJwtCreationAndIncludeClaims is a very long attribute name, a shorter name should be considered .

ADDRESSED
in #2391

CustomAttribute

  • API missing attribute displayValue
  • API missing attribute value

ADDRESSED
Reported issue fixed in Spec

OIDC Client

  • logoUri defined as string, but get object
  • clientUri defined as string, but get object
  • policyUri defined as string, but get object
  • tosUri defined as string, but get object
  • lastAccessTime defined as string, but get int
  • lastLogonTime defined as string, but get int
  • backchannelUserCodeParameter defined as boolean, but get string
  • API missing attribute authenticationMethod
  • API missing attribute tokenBindingSupported
  • API missing attribute baseDn
  • API missing attribute clientName

ADDRESSED
-logoUri: rectified to LocalizedString - had not done the change earlier as it was supposed to be changed back to string in auth
-clientUri: rectified to LocalizedString - had not done the change earlier as it was supposed to be changed back to string in auth
-policyUri: rectified to LocalizedString - had not done the change earlier as it was supposed to be changed back to string in auth
-tosUri: rectified to LocalizedString - had not done the change earlier as it was supposed to be changed back to string in auth
-lastAccessTime: no change done as date is defined in swagger as string of date-time format
-lastLogonTime : no change done as date is defined in swagger as string of date-time format
Following and other missing attributes added:
-authenticationMethod
-tokenBindingSupported
-baseDn
-clientName

NativePersistenceCacheConfiguration

  • API missing attribute disableAttemptUpdateBeforeInsert

Scope

  • API missing attribute baseDn

Organization

  • API missing attribute dn
  • API missing attribute baseDn
  • API missing attribute organizationTitle (maybe that's called title in the API, because that one is missing in the response)
  • API has attribute CountryName which doesn't exist in the backend
  • API has attribute Status which doesn't exist in the backend
  • API has attribute CustomMessages which doesn't exist in the backend
  • API has attribute Title which doesn't exist in the backend
  • API has attribute JsLogoPath which doesn't exist in the backend
  • API has attribute JsFaviconPath which doesn't exist in the backend

SMTP

  • Updating an existing SMTP only changes host, port, password fields. All other fields are not changed

LDAP configuration

  • Cannot change useSSL attribute

Couchbase configuration

  • Cannot update the attributes SSLTrustStoreFormat, OperationTracingEnabled, ComputationPoolSize
  • Many attributes are marked as required by the API, however, don't seem to be required and also it doesn't make sense for them to be required

All database configurations

  • whenever a get request is done on any of the database endpoints in the test system, I get a auth_ldap_server configID as a response, even if all the other fields are not set. This behaviour is not really clear, nor is it documented. What is the intention behind it?

AdminPermission

  • should naming be AdminPermission, AdminUIPermission, AdminUIUserPermission?
  • no get for individual permission, only possible to get all permissions at once. At the same time permissions can be individually updated or deleted.
  • which attribute should be the ID? permission?
  • ideally delete should only take the ID of the entity, e.g. permission

ADDRESSED
in #2390

AdminRole

  • same concerns as for AdminPermission

ADDRESSED
in #2390

AdminRolePermissionMapping

  • same concerns as for AdminPermission

AdminUI permission

  • API is missing attribute defaultPermissionInToken

ADDRESSED
in #2390

AgamaFlow

  • For newly created flows, it's not possible to retrieve the source attribute.
  • For newly created flows, the revision is set to -1, despite providing 1 as input

ANSWERED
in #2227 (comment)

CustomUser

  • When creating a new custom user, I get the following exception:
2022-09-14 12:52:15.706:WARN :oejs.HttpChannel:qtp762476028-14: /jans-config-api/mgt/configuser
org.jboss.resteasy.spi.UnhandledException: io.jans.orm.exception.EntryPersistenceException: Failed to persist entry: 'inum=7bbc68cc-3afe-47d1-b977-f7ebc58996d7,ou=people,o=jans'
	...
Caused by: 
io.jans.orm.exception.EntryPersistenceException: Failed to persist entry: 'inum=7bbc68cc-3afe-47d1-b977-f7ebc58996d7,ou=people,o=jans'
	at io.jans.orm.sql.impl.SqlEntryManager.persist(SqlEntryManager.java:218)
	...
	at io.jans.orm.PersistenceEntryManager$EntityManager$722412412$Proxy$_$$_WeldClientProxy.persist(Unknown Source)
	at io.jans.as.common.service.common.UserService.addUser(UserService.java:168)
	at io.jans.configapi.plugin.mgt.service.UserMgmtService$Proxy$_$$_WeldClientProxy.addUser(Unknown Source)
	at io.jans.configapi.plugin.mgt.rest.UserResource.createUser(UserResource.java:158)
	at io.jans.configapi.plugin.mgt.rest.UserResource$Proxy$_$$_WeldSubclass.createUser$$super(Unknown Source)
	...
Caused by: 
java.lang.NullPointerException
	at io.jans.orm.sql.operation.impl.SqlOperationServiceImpl.addEntryImpl(SqlOperationServiceImpl.java:185)
	at io.jans.orm.sql.operation.impl.SqlOperationServiceImpl.addEntry(SqlOperationServiceImpl.java:169)
	...
	at io.jans.orm.PersistenceEntryManager$EntityManager$722412412$Proxy$_$$_WeldClientProxy.persist(Unknown Source)
	at io.jans.as.common.service.common.UserService.addUser(UserService.java:168)
	at io.jans.configapi.plugin.mgt.service.UserMgmtService$Proxy$_$$_WeldClientProxy.addUser(Unknown Source)
	at io.jans.configapi.plugin.mgt.rest.UserResource.createUser(UserResource.java:158)
	at io.jans.configapi.plugin.mgt.rest.UserResource$Proxy$_$$_WeldSubclass.createUser$$super(Unknown Source)
	...
@moabu moabu assigned moabu and pujavs and unassigned moabu Aug 26, 2022
@mo-auto mo-auto added the kind-bug Issue or PR is a bug in existing functionality label Aug 26, 2022
@moabu moabu modified the milestones: 1.0.2, 1.0.3 Aug 29, 2022
@moabu moabu changed the title fix(config-api): issues in JansFido2DynConfiguration and attributes fix(config-api): issues in the API and attributes Aug 30, 2022
@yuriyz
Copy link
Contributor

yuriyz commented Sep 1, 2022

re: runIntrospectionScriptBeforeAccessTokenAsJwtCreationAndIncludeClaims. This long name come from AS. It was added a long time ago. Idea behind was to make clear what that property do. We can rename it to be shorter but still name should reflect what that "flag" switch on.

@jgomer2001
Copy link
Contributor

For newly created flows, it's not possible to retrieve the source attribute.

It's intentional. Read the docs to learn how to get source code. Maybe needs to be clarified somehow in yaml ?

For newly created flows, the revision is set to -1, despite providing 1 as input

It does not make sense to supply revision number for a new flow. -1 is expected

@nynymike
Copy link
Contributor

re: runIntrospectionScriptBeforeAccessTokenAsJwtCreationAndIncludeClaims ... it is a little long. Frankly, we now have the Update Token script. A little shorter: runIntrospectionScriptBeforeJwtCreation ?

@yuriyz
Copy link
Contributor

yuriyz commented Sep 14, 2022

Or maybe runIntrospectionScriptOnAccessTokenCreation to highlight it's about access token.

@nynymike
Copy link
Contributor

IMHO, It's implicit it's the access token because it's the introspection endpoint

@yuriyz
Copy link
Contributor

yuriyz commented Sep 14, 2022

Then agreed, I will rename it to runIntrospectionScriptBeforeJwtCreation.

@pujavs
Copy link
Contributor

pujavs commented Sep 15, 2022

JansFido2DynConfiguration:

Reported issue fixed in Spec

Attributes:

Reported issue fixed in Spec
Response is missing attribute - if values is null then that attribute is not part of response.
Few attributes were redundant hence removed from spec

CouchbaseConfig:

Reported issue fixed in Spec

Custom scripts:

Reported issue fixed in Spec

AuthConfig :

authorizationRequestCustomAllowedParameters returns array of AuthorizationRequestCustomParameter and spec is correct
image

NativePersistenceConfiguration:

Reported issue fixed in Spec

JWKs:

Modified DELETE on /jans-config-api/api/v1/config/jwks/{kid} to use https://jans.io/oauth/config/jwks.delete scope

UMAResource:

rev attribute removed as confirmed by @yuriyz

ClientAttributes:

Fixed by @yuriyz

CustomAttribute:

Reported missed attributes fixed in Spec

OIDC Client:

-logoUri: rectified to LocalizedString - had not done the change earlier as it was supposed to be changed back to string in auth
-clientUri: rectified to LocalizedString - had not done the change earlier as it was supposed to be changed back to string in auth
-policyUri: rectified to LocalizedString - had not done the change earlier as it was supposed to be changed back to string in auth
-tosUri: rectified to LocalizedString - had not done the change earlier as it was supposed to be changed back to string in auth
-lastAccessTime: no change done as date is defined in swagger as string of date-time format
-lastLogonTime : no change done as date is defined in swagger as string of date-time format
Following and other missing attributes added:
-authenticationMethod
-tokenBindingSupported
-baseDn
-clientName

Scope:

Reported missed attributes fixed in Spec

Organization:

Following and other missing attributes added:
dn
baseDn
organizationTitle
For extra attributes in API, that are not present in backend have requested @yuriyz to chk

SMTP:

Able to update all the fields in LDAP DB, pls see attached video.
JSON used: smtp_PUT.txt
Video: smtp_update.zip

LDAP configuration:

Able to update useSSL LDAP configuration in LDAP DB, pls see attached video.
Video: ldap_useSSL_update_test.zip

Couchbase configuration:

There are difference between jans-orm model https://github.com/JanssenProject/jans/blob/main/jans-orm/couchbase/src/main/java/io/jans/orm/couchbase/model/CouchbaseConnectionConfiguration.java and template that you shared https://github.com/JanssenProject/jans/blob/main/jans-linux-setup/jans_setup/templates/jans-couchbase.properties
As confirmed by @yurem confirmed that the CB endpoint in existing state might not be useful. Have asked him to guide on the same.
CB and MySql endpoint removed post discussion with @yurem via #2480

All database configurations:

Verified and it seems that test env configId for any DB is defined as auth_ldap_server.
Confirmed by @devrimyatar that the name is the same and regardless of db backend
https://github.com/JanssenProject/jans/blob/main/jans-linux-setup/jans_setup/templates/configuration.ldif#L4

@yuriyz
Copy link
Contributor

yuriyz commented Sep 15, 2022

Rev attribute is required, but not marked as such in the API. Not providing it results in a 500 error (it should be a 400). Also it should probably be of type int, as rev attributes in other entities are of type int and it seems to be parsed to an int by the server anyway.

Rev attribute is removed as redundant in #2393

@pujavs
Copy link
Contributor

pujavs commented Sep 27, 2022

CB and MySql endpoint removed post discussion with @yurem via #2480
Pending issue is CustomUser - working on the same

@pujavs
Copy link
Contributor

pujavs commented Sep 28, 2022

Tested with latest code on MySql jenkins env, not able to replicate CustomUser issue. Shared details with @moabu
No other issue pending

@pujavs
Copy link
Contributor

pujavs commented Oct 7, 2022

@moabu request your confirmation for closure

@moabu
Copy link
Member Author

moabu commented Oct 27, 2022

I'm closing and will create separate issues for the ones that remain

@moabu moabu closed this as completed Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind-bug Issue or PR is a bug in existing functionality
Projects
None yet
Development

No branches or pull requests

6 participants