Yair Zaslavsky has posted comments on this change. Change subject: aaa: InternalAuthenticator should use the new API ......................................................................
Patch Set 4: (6 comments) http://gerrit.ovirt.org/#/c/26443/4/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticatorProxy.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AuthenticatorProxy.java: Line 13: Line 14: private Extension extension; Line 15: private ExtMap context; Line 16: Line 17: public AuthenticatorProxy(Extension extension, ExtMap context) { > Oh.... get ExtensionProxy yes, the ctor should accept ExtensionProxy, i realize now. Line 18: this.extension = extension; Line 19: this.context = context; Line 20: } Line 21: Line 33: Authn.InvokeKeys.USER, user Line 34: ).mput( Line 35: Authn.InvokeKeys.CREDENTIALS, password Line 36: ); Line 37: ExtensionProxy proxy = new ExtensionProxy(extension, context); > no need to proxy again, the extension is already a proxy. yes, the CTOR of AuthenticatorProxy should be changed to accept ExtensionProxy. Line 38: ExtMap outputMap = proxy.invoke(inputMap); Line 39: if (outputMap.<Integer> get(Authn.InvokeKeys.RESULT) == Authn.AuthResult.CREDENTIALS_INVALID) { Line 40: throw new AAAExtensionException(AAAExtensionException.AAAExtensionError.INCORRECT_CREDENTIALS, ""); Line 41: } Line 36: ); Line 37: ExtensionProxy proxy = new ExtensionProxy(extension, context); Line 38: ExtMap outputMap = proxy.invoke(inputMap); Line 39: if (outputMap.<Integer> get(Authn.InvokeKeys.RESULT) == Authn.AuthResult.CREDENTIALS_INVALID) { Line 40: throw new AAAExtensionException(AAAExtensionException.AAAExtensionError.INCORRECT_CREDENTIALS, ""); > I would like you to use the Authn.AuthResult instead of the AAAExtensionExc IMHO this should be in next phases. I would like first to see the authenticator and directory working. Line 41: } Line 42: } Line 43: Line 44: @Override http://gerrit.ovirt.org/#/c/26443/4/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 26: doAuthenticate(input, output); Line 27: } Line 28: output.mput(Base.InvokeKeys.RESULT, Base.InvokeResult.SUCCESS); Line 29: } catch (Exception ex) { Line 30: output.mput(Base.InvokeKeys.RESULT, Base.InvokeResult.FAILED); > please also put message Done Line 31: } Line 32: Line 33: } Line 34: Line 32: Line 33: } Line 34: Line 35: private void doAuthenticate(ExtMap input, ExtMap output) { Line 36: String adminUser = configuration.getProperty("config.authn.user.name"); > please get all configuration variables into members during initialization, Done Line 37: Line 38: String adminPassword= input.<ExtMap> get( Line 39: Base.InvokeKeys.CONTEXT Line 40: ).<Properties>get(Base.ContextKeys.CONFIGURATION).getProperty("config.authn.user.password"); Line 52: mput(Base.ContextKeys.LICENSE, "ASL 2.0"). Line 53: mput(Base.ContextKeys.HOME_URL, "http://www.ovirt.org";). Line 54: mput(Base.ContextKeys.VERSION, "N/A"). Line 55: mput(Authn.ContextKeys.CAPABILITIES, Authn.Capabilities.AUTHENTICATE_PASSWORD); Line 56: configuration = context.<Properties> get(Base.ContextKeys.CONFIGURATION); > extract what needed and do not store configuration as a member. Done Line 57: } Line 58: -- 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: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[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
