Alon Bar-Lev 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";
> Hi,
I put the value that you currently use... I think... it is time to have our own 
namespace, no?

it should be done also in META-INF or other java logic?
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 =
> i'll fix the ident. not sure I understand your comment about "these classes
so why do you need to check for null?
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 65:                         + authnExtension.getName());
Line 66:             }
Line 67: 
Line 68:             AuthenticationProfile profile = new 
AuthenticationProfile(profileName, authenticator, directory);
Line 69:             profiles.put(profileName, profile);
> ok... if I understand it correctly, it all boils into:
can we do this ^ or there is a problem I do not see?
Line 70:         }
Line 71:     }
Line 72: 
Line 73:     /**


-- 
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