Alon Bar-Lev has posted comments on this change. Change subject: aaa: InternalAuthenticator should use the new API ......................................................................
Patch Set 3: (8 comments) http://gerrit.ovirt.org/#/c/26443/3/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalAuthenticator.java File backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalAuthenticator.java: Line 7: import org.ovirt.engine.api.extensions.Extension; Line 8: import org.ovirt.engine.api.extensions.aaa.Authn; Line 9: import org.ovirt.engine.api.extensionsold.AAAExtensionException; Line 10: import org.ovirt.engine.core.aaa.Authenticator; Line 11: import org.ovirt.engine.core.extensions.mgr.ExtensionProxy; this class should not use anything of core, only api Line 12: Line 13: /** Line 14: * This authenticator authenticates the internal user as specified in the {@code AdminUser} and {@code AdminPassword} Line 15: * configuration parameters stored in the database. Currently it is in an interim status of development as Line 15: * configuration parameters stored in the database. Currently it is in an interim status of development as Line 16: */ Line 17: public class InternalAuthenticator extends Authenticator implements Extension { Line 18: Line 19: private ExtMap initMap; please use consistent terms... this is context. Line 20: Line 21: // This method should be removed once we no longer work with Authenticator class hierarchy Line 22: @Override Line 23: @Deprecated Line 48: doInit(input, output); Line 49: } Line 50: if (input.get(Base.InvokeKeys.COMMAND).equals(Authn.InvokeCommands.AUTHENTICATE_CREDENTIALS)) { Line 51: doAuthenticate(input, output); Line 52: } I suggest try catch and set success/failure here. Line 53: } Line 54: Line 55: @Override Line 56: public String getName() { Line 51: doAuthenticate(input, output); Line 52: } Line 53: } Line 54: Line 55: @Override this depreciated as well, right? Line 56: public String getName() { Line 57: return (String) initMap.<ExtMap> get(Base.InvokeKeys.CONTEXT).<String> get(Base.ContextKeys.INSTANCE_NAME); Line 58: } Line 59: Line 56: public String getName() { Line 57: return (String) initMap.<ExtMap> get(Base.InvokeKeys.CONTEXT).<String> get(Base.ContextKeys.INSTANCE_NAME); Line 58: } Line 59: Line 60: @Override depreciated as well, right? Line 61: public String getProfileName() { Line 62: return (String) initMap.<ExtMap> get(Base.InvokeKeys.CONTEXT) Line 63: .<Properties> get(Base.ContextKeys.CONFIGURATION) Line 64: .getProperty("ovirt.engine.aaa.authn.profile.name"); Line 64: .getProperty("ovirt.engine.aaa.authn.profile.name"); Line 65: } Line 66: Line 67: private void doAuthenticate(ExtMap input, ExtMap output) { Line 68: String adminUser = input.<ExtMap> get( you already have context as member... so either you use it always or use the input? Line 69: Base.InvokeKeys.CONTEXT Line 70: ).<Properties>get(Base.ContextKeys.CONFIGURATION).getProperty("config.authn.user.name"); Line 71: Line 72: String adminPassword= input.<ExtMap> get( Line 66: Line 67: private void doAuthenticate(ExtMap input, ExtMap output) { Line 68: String adminUser = input.<ExtMap> get( Line 69: Base.InvokeKeys.CONTEXT Line 70: ).<Properties>get(Base.ContextKeys.CONFIGURATION).getProperty("config.authn.user.name"); get this during initialization, no need to get configuration each time. once initialized you should not look into config. Line 71: Line 72: String adminPassword= input.<ExtMap> get( Line 73: Base.InvokeKeys.CONTEXT Line 74: ).<Properties>get(Base.ContextKeys.CONFIGURATION).getProperty("config.authn.user.password"); Line 85: mput(Base.ContextKeys.LICENSE, "ASL 2.0"). Line 86: mput(Base.ContextKeys.HOME_URL, "http://www.ovirt.org";). Line 87: mput(Base.ContextKeys.VERSION, "N/A"). Line 88: mput(Authn.ContextKeys.CAPABILITIES, Authn.Capabilities.AUTHENTICATE_PASSWORD); Line 89: initMap = input; you should store the context, not the input. input should be allowed to be freed. Line 90: output.mput(Base.InvokeKeys.RESULT, Base.InvokeResult.SUCCESS); Line 91: } Line 92: -- To view, visit http://gerrit.ovirt.org/26443 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I60f7b7f50617bff9f4872dc79f14fb016c9d72d3 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: [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
