Alon Bar-Lev has posted comments on this change. Change subject: aaa: InternalAuthenticator should use the new API ......................................................................
Patch Set 4: (8 comments) http://gerrit.ovirt.org/#/c/26443/4/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 81: Line 82: for (ExtensionProxy authnExtension : ExtensionsManager.getInstance().getProvidedExtensions(AUTHN_SERVICE)) { Line 83: registerProfile( Line 84: new AuthenticationProfile( Line 85: new AuthenticatorProxy(authnExtension, authnExtension.getContext()), if you have authnExtension you do not need the context as you can retrieve it. Line 86: (Directory) (Extension) ExtensionsManager.getInstance().getExtensionByName( Line 87: authnExtension.getContext().<Properties>get(Base.ContextKeys.CONFIGURATION).getProperty( Line 88: AUTHN_AUTHZ_PLUGIN Line 89: ) 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 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. 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 34: ).mput( Line 35: Authn.InvokeKeys.CREDENTIALS, password Line 36: ); Line 37: ExtensionProxy proxy = new ExtensionProxy(extension, context); Line 38: ExtMap outputMap = proxy.invoke(inputMap); no need to use temporary variable for 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 42: } 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 AAAExtensionException and its values. 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 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, it is not that important, but I think will make code easier to read. 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. 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
