Martin Peřina has posted comments on this change.
Change subject: core: Introduce new authentication interfaces
......................................................................
Patch Set 8:
(2 comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/authentication/AuthenticationFilter.java
Line 34: private static final String STACK_ATTR =
AuthenticationFilter.class.getName() + ".stack";
Line 35:
Line 36: @Override
Line 37: public void init(FilterConfig filterConfig) throws
ServletException {
Line 38: // Nothing.
Well, comment "nothing" doesn't have any meaning for me. I would leave it
without any comment, I think it's clear that it's implemented here only to
satisfy implementing Filter interface.
But if you want really to mark this as "no code here by purpose", please add
Javadoc, that method is empty.
Line 39: }
Line 40:
Line 41: @Override
Line 42: public void destroy() {
Line 118: // If the negotiation is finished and authentication
succeeded then we have to remember in the session that
Line 119: // the user has been authenticated and its login name,
also we need to clean the stack of authenticators and
Line 120: // replace the request with a wrapper that contains the
user name returned by the authenticator:
Line 121: if (result.isAuthenticated()) {
Line 122: String name = result.getName() + "@" +
profile.getName();
Please take a look at this:
http://www.javacodegeeks.com/2013/03/java-stringbuilder-myth-debunked.html
I would change the code to:
String name = new StringBuilder(result.getName)
.append("@")
.append(profile.getName)
.toString();
But since you have here only two concats, you may be right, that it's not worth
it.
Line 123: session.setAttribute(AUTHENTICATED_ATTR, true);
Line 124: session.setAttribute(NAME_ATTR, name);
Line 125: session.removeAttribute(STACK_ATTR);
Line 126: req = new AuthenticatedRequestWrapper(req, name);
--
To view, visit http://gerrit.ovirt.org/15596
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If84a0c9d6553d81cdbbe224972696f169cca90d4
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Martin Peřina <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: mooli tayer <[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
