Martin Peřina has posted comments on this change.
Change subject: core: Introduce new authentication interfaces
......................................................................
Patch Set 7:
(6 comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/authentication/AuthenticationFilter.java
Line 17: /**
Line 18: * This filter should be added to the {@code web.xml} file to the
applications that wan't to use the authentication
Line 19: * mechanism implemented in this package.
Line 20: */
Line 21: public class AuthenticationFilter implements Filter {
Please convert comments to JavaDoc for attributes and methods
Line 22: // The authenticator used to perform the authentication process:
Line 23: private List<NegotiatingAuthenticator> authenticators;
Line 24:
Line 25: // We store a boolean flag in the HTTP session that indicates if
the user has been already authenticated, this is
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/authentication/AuthenticationProfileFactory.java
Line 2:
Line 3: import java.io.File;
Line 4:
Line 5: import org.slf4j.Logger;
Line 6: import org.slf4j.LoggerFactory;
I thought org.ovirt.engine.core.utils.log is preferred logging framework for
ovirt
Line 7:
Line 8: /**
Line 9: * An authentication profile is just a pair composed of an
authenticator and a directory so this class just looks up
Line 10: * those two services when the profile is created.
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/authentication/AuthenticationProfileManager.java
Line 21: }
Line 22: }
Line 23: }
Line 24: return manager;
Line 25: }
Wouldn't be better to implement singleton using enum?
Line 26:
Line 27: private AuthenticationProfileManager() {
Line 28: super(AuthenticationProfileFactory.class);
Line 29: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/authentication/AuthenticatorManager.java
Line 21: }
Line 22: }
Line 23: }
Line 24: return instance;
Line 25: }
Woudn't it be better to implement signleton as an enum?
Line 26:
Line 27: private AuthenticatorManager() {
Line 28: super(AuthenticatorFactory.class);
Line 29: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/authentication/Configuration.java
Line 23: public static Configuration load(File file) throws IOException {
Line 24: Properties properties = new Properties();
Line 25: InputStream in = null;
Line 26: try {
Line 27: in = new FileInputStream(file);
Please use try-with-resources
Line 28: properties.load(in);
Line 29: }
Line 30: finally {
Line 31: in.close();
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/authentication/DirectoryManager.java
Line 22: }
Line 23: }
Line 24: }
Line 25: return instance;
Line 26: }
Wouldn't it be better to implement singleton using enum?
Line 27:
Line 28: private DirectoryManager() {
Line 29: super(DirectoryFactory.class);
Line 30: }
--
To view, visit http://gerrit.ovirt.org/15596
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If84a0c9d6553d81cdbbe224972696f169cca90d4
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Martin Peřina <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
