Vojtech Szocs has posted comments on this change.

Change subject: webadmin: reports sso token
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.ovirt.org/#/c/27542/4/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/system/BaseApplicationInit.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/system/BaseApplicationInit.java:

Line 172:                 getLoginModel().autoLogin(loggedUser);
Line 173:             }
Line 174:         });
Line 175: 
Line 176:         SSOTokenChangeEvent.fire(eventBus, 
SSOTokenData.instance().getSsoToken());
> No, if the value is not available, the reports will simply be disabled, but
I agree that we should fire SSOTokenChangeEvent even when we don't have SSO 
token, I was just wondering about the case when:

 SSOTokenData.instance()

might return null due to missing "ssoToken" global object, which could result 
in NPE and break the login process.

Thinking about it, above concern is not very likely to happen, IIUC we always 
embed "ssoToken" global object into GWT host page.
Line 177:         // Indicate that the user should be logged in automatically
Line 178:         user.setAutoLogin(true);
Line 179:     }
Line 180: 


http://gerrit.ovirt.org/#/c/27542/4/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/ReportInit.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/ReportInit.java:

Line 180:             new SSOTokenChangeHandler() {
Line 181: 
Line 182:                 @Override
Line 183:                 public void onSSOTokenChange(SSOTokenChangeEvent 
event) {
Line 184:                     ssoToken = event.getSsoToken();
> If it is null, I am setting it to "" so the login will continue will the re
Hm, but in checkIfInitFinished method we have this:

 if (xmlInitialized && urlInitialized && ssoToken != null) {
     ...
 }

so we should probably update that condition? (I don't see where we set it to 
empty string, I am probably missing something.)

Maybe we can simply return empty string right in SSOTokenData.getToken method, 
if actual token value is null.
Line 185:                     checkIfInitFinished();
Line 186:                 }
Line 187:             }
Line 188:         );


-- 
To view, visit http://gerrit.ovirt.org/27542
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib906a1cca87712fb4e31c68c5e8c70151976ce0f
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Yaniv Dary <[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

Reply via email to