Alon Bar-Lev has posted comments on this change. Change subject: tools: extensions-tool: initial implementation aaa ......................................................................
Patch Set 2: (12 comments) https://gerrit.ovirt.org/#/c/41605/2/backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/aaa/AAAServiceImpl.java File backend/manager/extension-tool/src/main/java/org/ovirt/engine/exttool/aaa/AAAServiceImpl.java: Line 154: Line 155: private void authzFetchPrincipalRecord() { Line 156: logger.debug("start authz_fetch_principal_record"); Line 157: Line 158: ExtensionProxy authzExtension = getExtensionByName((String) this.actionArgs.get("extension-name")); why do you need this.? Line 159: Line 160: ExtMap outMap = authzExtension.invoke( Line 161: new ExtMap().mput( Line 162: Authz.InvokeKeys.PRINCIPAL, Line 195: password Line 196: ) Line 197: ); Line 198: ExtMap authRecord = processAuthnResult(outMap, "AUTHENTICATE_CREDENTIALS"); Line 199: if(logout) { not sure this belongs here. 1. you can skip logout if not in complete sequence. 2. caller should call logout. Line 200: logoutUser(authRecord, authnExtension); Line 201: } Line 202: Line 203: logger.info("authn_authenticate_credentials action completed"); Line 215: authnExtension.getContext().<Properties>get(Base.ContextKeys.CONFIGURATION).getProperty(Authn.ConfigKeys.MAPPING_PLUGIN) Line 216: ); Line 217: Line 218: // acc Line 219: String user = (String)this.actionArgs.get("principal-name"); in login this should be user name not principal name Line 220: if(mappingExtension != null) { Line 221: user = mappingExtension.invoke( Line 222: new ExtMap().mput( Line 223: Base.InvokeKeys.COMMAND, Line 229: true Line 230: ).<String>get(Mapping.InvokeKeys.USER, user); Line 231: } Line 232: Line 233: // authn log/print user name after mapping? Line 234: ExtMap authRecord = authnAuthenticateCredentials(authnExtension, false); Line 235: Line 236: // acc Line 237: if(mappingExtension != null) { Line 230: ).<String>get(Mapping.InvokeKeys.USER, user); Line 231: } Line 232: Line 233: // authn Line 234: ExtMap authRecord = authnAuthenticateCredentials(authnExtension, false); log/print authn record? Line 235: Line 236: // acc Line 237: if(mappingExtension != null) { Line 238: user = mappingExtension.invoke( Line 237: if(mappingExtension != null) { Line 238: user = mappingExtension.invoke( Line 239: new ExtMap().mput( Line 240: Base.InvokeKeys.COMMAND, Line 241: Mapping.InvokeCommands.MAP_USER this should be MAP_AUTH_RECORD and provided auth record. Line 242: ).mput( Line 243: Mapping.InvokeKeys.USER, Line 244: user Line 245: ), Line 244: user Line 245: ), Line 246: true Line 247: ).<String>get(Mapping.InvokeKeys.USER, user); Line 248: } log/print authn record after mapping? Line 249: Line 250: /// authz Line 251: if (authzExtension != null) { Line 252: authzExtension.invoke(new ExtMap(). Line 261: getFlags() Line 262: ) Line 263: ); Line 264: } Line 265: // 5. acct - TODO: log/print authz record? Line 266: Line 267: // logout Line 268: logoutUser(authRecord, authnExtension); Line 269: Line 277: getExtensionByName((String)this.actionArgs.get("authz-name")) Line 278: ); Line 279: } catch (Exception ex) { Line 280: throw new RuntimeException(ex); Line 281: } do we use anything else but RuntimeException? Line 282: logger.info("search action completed"); Line 283: } Line 284: Line 285: private ExtMap logoutUser(ExtMap authRecord, ExtensionProxy authnExtension) { Line 375: throw new IllegalArgumentException(String.format("Profile '%1$s' not found")); Line 376: } Line 377: Line 378: private ExtensionProxy getExtensionByName(String name) { Line 379: return ((Map<String, ExtensionProxy>) context.get(EXTENSIONS_MAP)).get(name); why don't you use the extensions manager? Line 380: } Line 381: Line 382: private ExtUUID getEntity() throws NoSuchFieldException, IllegalAccessException { Line 383: return (ExtUUID)Authz.QueryEntity.class.getDeclaredField( https://gerrit.ovirt.org/#/c/41605/2/backend/manager/extension-tool/src/main/resources/org/ovirt/engine/exttool/aaa/arguments.properties File backend/manager/extension-tool/src/main/resources/org/ovirt/engine/exttool/aaa/arguments.properties: Line 94: login-user.arg.resolve-groups.name = resolve-groups Line 95: login-user.arg.resolve-groups.help = Resolve groups Line 96: login-user.arg.resolve-groups.valuetype = java.lang.Integer Line 97: login-user.arg.resolve-groups.value = 1 Line 98: login-user.arg.resolve-groups.default = 0 shouldn't this be flags? Line 99: Line 100: login-user.arg.resolve-groups-recursive.name = resolve-groups-recursive Line 101: login-user.arg.resolve-groups-recursive.help = Resolve groups recursive Line 102: login-user.arg.resolve-groups-recursive.valuetype = java.lang.Integer Line 100: login-user.arg.resolve-groups-recursive.name = resolve-groups-recursive Line 101: login-user.arg.resolve-groups-recursive.help = Resolve groups recursive Line 102: login-user.arg.resolve-groups-recursive.valuetype = java.lang.Integer Line 103: login-user.arg.resolve-groups-recursive.value = 2 Line 104: login-user.arg.resolve-groups-recursive.default = 0 shouldn't this be flags? Line 105: Line 106: login-user.help.header = Login action of aaa interface test module Line 107: login-user.help.usage = @PROGRAM_NAME@ aaa login [options] Line 108: login-user.help.footer = \ -- To view, visit https://gerrit.ovirt.org/41605 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1811c5845bf02c30f1acd2938074070fb661af38 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ondra Machacek <omach...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondra Machacek <omach...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
