Yair Zaslavsky has posted comments on this change.

Change subject: 6. [WIP] core: Introducing AuthenticationProfileRepository
......................................................................


Patch Set 9:

(3 comments)

http://gerrit.ovirt.org/#/c/24366/9/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticationProfileRepository.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticationProfileRepository.java:

Line 11: 
Line 12: public class AuthenticationProfileRepository {
Line 13: 
Line 14:     private static final String AUTHN_PROFILE = 
"ovirt.engine.aaa.authn.profile.name";
Line 15:     private static final String AUTHN_SERVICE = 
"org.ovirt.engine.core.authentication";
> please rename services to org.ovirt.engine.extension.services.xxxx
Hi,
This is the value of the key - ovirt.engine.extension.service
 at the authentication configuration. In your example - this is the value 
(well, you set some question marks, I guess you were not sure).
Please suggest if you want it to be fixed.
Line 16:     private static final String AUTHN_AUTHZ_PLUGIN = 
"ovirt.engine.aaa.authn.authz.plugin";
Line 17: 
Line 18: 
Line 19:     private static volatile AuthenticationProfileRepository instance = 
null;


Line 48: 
Line 49:             }
Line 50: 
Line 51:             Directory directory = 
DirectoryManager.getInstance().parseDirectory(authzExtension.getConfig());
Line 52:                 Authenticator authenticator =
> why the indent?
i'll fix the ident. not sure I understand your comment about "these classes do 
not throw exception" - both DirectoryManager and AuthenticationManager do throw 
exceptions - as ConfigurationException extends RuntimeException it might 
"miseld" you during the reading of the code.
If you wish I can add "throws ConfigurationException" at parseDirecctory and 
parseAuthenticator.
Line 53:                     
AuthenticatorManager.getInstance().parseAuthenticator(authzExtension.getConfig());
Line 54:             if (directory == null) {
Line 55:                 throw new ConfigurationException("No directory could 
be obtained for authorization plugin "
Line 56:                         + authzExtension.getName());


Line 97:      *            the profile to register
Line 98:      */
Line 99:     public void registerProfile(String name, AuthenticationProfile 
profile) {
Line 100:         profiles.put(name, profile);
Line 101:     }
> why do we need register? where can it come from? name should be acquired fr
interim method.
will be removed later on (see usage , but i will add a comment).


-- 
To view, visit http://gerrit.ovirt.org/24366
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If375fccea98544c64d9ec41cc9dbcb855bf02fb7
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Martin Peřina <mper...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to