Juan Hernandez has posted comments on this change.
Change subject: core: Introduce new authentication interfaces
......................................................................
Patch Set 8:
(9 comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/authentication/AuthenticatedRequestWrapper.java
Line 4: import javax.servlet.http.HttpServletRequest;
Line 5: import javax.servlet.http.HttpServletRequestWrapper;
Line 6:
Line 7: /**
Line 8: * This class wraps a HTTP request when the authentication process has
suceeded in order to replace the principal used
Done
Line 9: * by default by the container with one containing the name of the user
that has been authenticated by the our own
Line 10: * authentication mechanism.
Line 11: */
Line 12: public class AuthenticatedRequestWrapper extends
HttpServletRequestWrapper {
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/authentication/AuthenticationFilter.java
Line 14: import javax.servlet.http.HttpServletResponse;
Line 15: import javax.servlet.http.HttpSession;
Line 16:
Line 17: /**
Line 18: * This filter should be added to the {@code web.xml} file to the
applications that wan't to use the authentication
Done
Line 19: * mechanism implemented in this package.
Line 20: */
Line 21: public class AuthenticationFilter implements Filter {
Line 22: // The authentication profiles used to perform the authentication
process:
Line 25: // We store a boolean flag in the HTTP session that indicates if
the user has been already authenticated, this is
Line 26: // the key for that flag:
Line 27: private static final String AUTHENTICATED_ATTR =
AuthenticationFilter.class.getName() + ".authenticated";
Line 28:
Line 29: // When an user has been authenticated we store its login name in
the HTTP session, this is the key for that name:
Mooli, my English grammar skills are really poor, thanks. Done.
Line 30: private static final String NAME_ATTR =
AuthenticationFilter.class.getName() + ".name";
Line 31:
Line 32: // In order to support several alternative authenticators we store
their names in a stack inside the HTTP session,
Line 33: // this is the key for that stack:
Line 39: }
Line 40:
Line 41: @Override
Line 42: public void destroy() {
Line 43: // Nothing.
Done
Line 44: }
Line 45:
Line 46: /**
Line 47: * Lazyly find all the profiles that that support negotiation and
store them reversed to simplify the creation of
Line 70: // authentication to perform:
Line 71: findNegotiatingProfiles();
Line 72: if (profiles.isEmpty()) {
Line 73: chain.doFilter(req, rsp);
Line 74: return;
Note that the administrator of the engine may have selected to use a password
authenticator instead of a negotiating authenticator. In that case we need to
let the request go to the application, so that it can show the user/password
dialog and perform the authentication.
Line 75: }
Line 76:
Line 77: // Perform the authentication:
Line 78: doFilter((HttpServletRequest) req, (HttpServletResponse) rsp,
chain);
Line 99: stack = new Stack<String>();
Line 100: for (AuthenticationProfile profile : profiles) {
Line 101: stack.push(profile.getName());
Line 102: }
Line 103: session.setAttribute(STACK_ATTR, stack);
Mooli is right.
Line 104: }
Line 105:
Line 106: while (!stack.isEmpty()) {
Line 107: // Resume the negotiation with the profile at the top of
the stack:
Line 111:
Line 112: // If the negotiation isn't finished then we assume that
the response has been populated by the
Line 113: // authenticator and we just let the container sent it
back to the client:
Line 114: if (result == null) {
Line 115: return;
It does interfere, and that is on purpose. The idea is that if the
authentication is in progress and being handled by an authenticator then the
request shouldn't make any further progress in the filter chain. It is the
responsibility of that authenticator to populate the response as required.
From the point of view of the user will look as the authenticator determines.
For a Basic authenticator, for example, the user won't notice anything because
the browser handles it automatically.
Imagine that we want to create an authenticator that shows to the user a page
with a captcha in order to perform login. This authenticator will be
responsible for generating that HTML page. This is why the authenticator
receives the request and response parameters, it can do anything a servlet can
do, including generating HTML pages. The user will see that page, fill the
form, and submit it. The submitted data will be processed again by the
authenticator, and if the form is correct it will finish the authentication,
returning something different to null.
All we can do here is remember what authenticator is performing the
authentication, that is why we have the stack. If the authenticator itself
needs to remember something it should store it in the session, in the URL as
parameter, in a hidden field, etc.
Line 116: }
Line 117:
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 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();
You both have good points. In order to settle, and being lazy, I will keep what
I currently have.
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);
Line 137: // If we are here then there are no more authenticators to
try so we need to invalidate the session and reject
Line 138: // the request:
Line 139: session.invalidate();
Line 140: rsp.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
Line 141: }
Yes Mooli, it is used here: http://gerrit.ovirt.org/21024
--
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: Alexander Wels <[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