Project

General

Profile

Actions

Task #2046

closed

Authentication against multiple systems

Added by Alena Peterová about 4 years ago. Updated almost 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Ondřej Kopr
Category:
Systems
Target version:
Start date:
02/10/2020
Due date:
% Done:

100%

Estimated time:
Owner:

Description

Please support authentication against multiple systems.
Now the authentication by https://wiki.czechidm.com/devel/documentation/security/dev/authentication#defaultaccauthenticator supports only one value in the property ''idm.sec.security.auth.systemId''. Please extend it to be multivalued.

Use-case:
  • there are identities from multiple companies in IdM
  • every company has its own Active Directory, which are connected as separated systems in IdM
  • identities should be able to authenticate with their account regardless of their company

Related issues

Related to IdStory Identity Manager - Feature #2295: Optimalization of authentication against end system - perform GET only if "Authentication attr." is checkedClosedRoman Kučera06/04/2020

Actions
Actions #2

Updated by Ondřej Kopr almost 4 years ago

  • Status changed from New to In Progress
  • Assignee changed from Vít Švanda to Ondřej Kopr

I take this ticket. For first will be created request and then maybe will be this implemented in extras module.

Actions #4

Updated by Ondřej Kopr almost 4 years ago

  • Tracker changed from Feature to Task
  • Project changed from IdStory Identity Manager to extras
  • Category deleted (Authentication / Authorization)
Actions #5

Updated by Ondřej Kopr almost 4 years ago

  • % Done changed from 0 to 10

First implementation exists.

Now is possible define infinite system with configuration properties. Against these system will be made authentication. Example of configuration properties:

idm.sec.extras.security.auth.order10.systemId=addc7c0-086b-44b5-9d0a-8dd265c16553
idm.sec.extras.security.auth.order11.systemId=ea86a399-9b26-4f75-9b3a-d3f0049031ef
idm.sec.extras.security.auth.order12.systemId=eb24ee1e-da69-4e8a-8b7d-8e41691f09e4
idm.sec.extras.security.auth.order13.systemId=e6a8b1e7-d656-47ae-aa2d-1062d1583c1a
idm.sec.extras.security.auth.order14.systemId=ecb6633a-088d-4021-ad56-5c22fb7a2b7f
idm.sec.extras.security.auth.order15.systemId=71fcccb6-5865-48e6-8fba-3d4d963d9f45

Actions #6

Updated by Ondřej Kopr almost 4 years ago

  • Related to Feature #2295: Optimalization of authentication against end system - perform GET only if "Authentication attr." is checked added
Actions #7

Updated by Vít Švanda almost 4 years ago

  • Project changed from extras to IdStory Identity Manager
  • Category set to Systems
  • Target version set to 10.4.0
Actions #8

Updated by Ondřej Kopr almost 4 years ago

  • % Done changed from 10 to 80

Implementation was finished.

Changes:
  • all behavior with authentication against system was moved to new abstract class - AbstractAccAuthenticator,
  • throws from authentication was removed - changed old behavior.
Newly implemented:
  • new authenticator - DefaultAccMultipleSystemAuthenticator (SUFFICIENT) (order is after old authenticator),
  • low ux desing,
  • new configuration class for acc authenticators - AuthenticatorConfiguration,
  • new configuration options with acc module prefix - old configuration property still exists.

Branch: https://github.com/bcvsolutions/CzechIdMng/tree/okopr/2046-authentication-against-multiple-systems

Documentation missing.

Actions #10

Updated by Ondřej Kopr almost 4 years ago

  • Status changed from In Progress to Needs feedback
  • Assignee changed from Ondřej Kopr to Radek Tomiška
Actions #11

Updated by Radek Tomiška almost 4 years ago

  • Status changed from Needs feedback to In Progress
  • Assignee changed from Radek Tomiška to Ondřej Kopr

I did test and code review, it works, thx! Code looks nice too (except ... you know :))

Minor review notes:
- AuthenticatorConfiguration - prevent to append configuration property from other constant - use configuration property as one string (configuration property can be found by ctrl-cv from documentation).
- AuthenticatorConfiguration - I like the property uses module prefix now, awesome! Maybe "systemId" can be ranamed to "system" => codeable is supported here.
- AuthenticatorConfiguration - SysSystemDto should be returned instead raw Strings (e.g. RoleConfiguration#getAdminRole) - now all callers has to check and resolve wrong configuration.
- DefaultAccAuthenticator could be deprecated - new DefaultAccMultipleSystemAuthenticator fully cover this feature.
- AbstractAccAuthenticator#161 - comment "Warning: There is silent error. Because one user can has more system and we don't fill log with errors." should be removed now? :)
- DefaultAccMultipleSystemAuthenticator#45 - comment "Before CORE module and after original ACC module. Original authenticator has bigger priority than this authenticator." - the core module is first, then this, old at the end (it's reversed). :)
- DefaultAuthenticatorConfiguration#65 - DEFAULT_AUTH_MAX_SYSTEM_COUNT is "long" - check for null value ca be removed (+ better coverage)

Actions #12

Updated by Ondřej Kopr almost 4 years ago

  • Status changed from In Progress to Needs feedback
  • Assignee changed from Ondřej Kopr to Radek Tomiška

Except the first composable property :D:

There are result from review notes:
  • prevent to append configuration property - I little bit changed behavior with append but there must be still two properties,
  • "systemId" can be ranamed to "system" - renamed,
  • SysSystemDto should be returned - both method in AuthenticatorConfiguration now return SysSystemDto,
  • deprecated DefaultAccAuthenticator - the DefaultAccAuthenticator is now deprecated with some information about new usage,
  • AbstractAccAuthenticator#161 - comment - comment was removed,
  • DefaultAccMultipleSystemAuthenticator#45 - comment the order was little bit changed and comments changed,
  • DEFAULT_AUTH_MAX_SYSTEM_COUNT is "long" check for null was removed and now is return directly result form getLong.
Thank you for review. There is commit: Changed documentation:

Please could you check the result? Thank you :)

Actions #13

Updated by Radek Tomiška almost 4 years ago

  • Status changed from Needs feedback to Resolved
  • Assignee changed from Radek Tomiška to Ondřej Kopr
  • % Done changed from 90 to 100

Thx! Merged into develop.

Actions #14

Updated by Radek Tomiška almost 4 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF