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

Update authenticator and idp management APIs #5140

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Thisara-Welmilla
Copy link
Contributor

Issue:

Update the API docs on follwoing endpoints:

  • /v1/authenticators
  • /v1/configs/authenticators
  • /v1/identity-provider

@Thisara-Welmilla Thisara-Welmilla marked this pull request as ready for review March 4, 2025 12:48
@Thisara-Welmilla
Copy link
Contributor Author

Thisara-Welmilla commented Mar 4, 2025

  • /v1/authenticators
Screenshot 2025-03-04 at 18 41 59 Screenshot 2025-03-04 at 18 42 06 Screenshot 2025-03-04 at 18 42 31 Screenshot 2025-03-04 at 18 42 53
  • /v1/configs/authenticators
Screenshot 2025-03-04 at 18 45 40
  • /v1/identity-provider
Screenshot 2025-03-04 at 18 57 58 Screenshot 2025-03-04 at 18 58 40 Screenshot 2025-03-04 at 18 58 50 Screenshot 2025-03-04 at 18 58 56 Screenshot 2025-03-04 at 18 59 05 Screenshot 2025-03-04 at 19 01 08

id: "Y3VzdG9tQXV0aGVudGljYXRvcg"
name: "customAuthenticator"
displayName: "Custom auth"
definedBy: "SYSTEM"
Copy link
Member

Choose a reason for hiding this comment

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

This should be 'USER' right ?

id: "QmFzaWNBdXRoZW50aWNhdG9y"
name: "BasicAuthenticator"
displayName: "basic"
definedBy: "USER"
Copy link
Member

Choose a reason for hiding this comment

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

This should be SYSTEM

definedBy: "USER"
type: "LOCAL"
isEnabled: true
tags: [ Custom, 2FA ]
Copy link
Member

Choose a reason for hiding this comment

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

These tags should go to the previous

properties:
username: "auth_username"
password: "auth_password"
SystemDefinedAuthenticatorExample:
Copy link
Member

Choose a reason for hiding this comment

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

Also this sample doesn't seem to apply at /custom path
isn't it ?

- isEnabled
- endpoint
UserDefinedLocalAuthenticatorUpdate:
description: TThis represents the configuration for updating user defined local authenticator.
Copy link
Member

Choose a reason for hiding this comment

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

typo

@@ -1239,9 +1283,20 @@ components:
displayName:
type: string
example: basic
description:
type: string
example: "Description for user defined local authenticator configuration."
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this model commonly used for both system defined and user defined

@@ -1208,10 +1241,21 @@ components:
displayName:
type: string
example: basic
description:
type: string
example: "Description for user defined local authenticator configuration."
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this model commonly used for both user defined and system defined ? I don't think we can give this description in that case

Comment on lines +3469 to +3473
description: "IdP for Google Federation"
image: "google-logo-url"
templateId: "8ea23303-49c0-4253-b81f-82c0fe6fb4a0"
isPrimary: false
Copy link
Member

Choose a reason for hiding this comment

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

This should be something custom

Comment on lines +3542 to +3549
name: "Custom IDP"
description: "IDP with custom federated authenticator"
image: "https://custom-authenticator-logo-url"
templateId: "8ea23303-49c0-4253-b81f-82c0fe6fb4a0"
isPrimary: false
isFederationHub: false
homeRealmIdentifier: ""
Copy link
Member

Choose a reason for hiding this comment

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

This is for system defined IdP. Looks like you have swapped the content of two examples

userstore: "PRIMARY"
associateLocalUser: true
attributeSyncMethod: "OVERRIDE_ALL"
Copy link
Member

Choose a reason for hiding this comment

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

Add new line

id: "Y3VzdG9tQXV0aGVudGljYXRvcg"
name: "customAuthenticator"
displayName: "Custom auth"
definedBy: "SYSTEM"
Copy link
Member

Choose a reason for hiding this comment

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

This should be USER

id: "QmFzaWNBdXRoZW50aWNhdG9y"
name: "BasicAuthenticator"
displayName: "basic"
definedBy: "USER"
Copy link
Member

Choose a reason for hiding this comment

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

This should be SYSTEM

@@ -3280,4 +3389,191 @@ components:
file:
type: string
format: binary
description: file to upload
Copy link
Member

Choose a reason for hiding this comment

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

Check on this line character

authenticatorId: "Y3VzdG9tQXV0aGVudGljYXRvcg"
name: "customAuthenticator"
definedBy: "SYSTEM"
Copy link
Member

Choose a reason for hiding this comment

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

This should be USER

value:
authenticatorId: "U0FNTDJBdXRoZW50aWNhdG9y"
definedBy: "USER"
Copy link
Member

Choose a reason for hiding this comment

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

This should be SYSTEM

userstore: "PRIMARY"
associateLocalUser: true
attributeSyncMethod: "OVERRIDE_ALL"
Copy link
Member

Choose a reason for hiding this comment

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

Add new line

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