Skip to end of metadata
Go to start of metadata

You are viewing an old version of this page. View the current version.

Compare with Current View Page History

« Previous Version 52 Current »

[12:38:25 CST(-0600)] <EricDalquist> uhg

[12:38:37 CST(-0600)] <EricDalquist> athena: it looks like the spring security servlet filter breaks remote_user based auth

[12:39:00 CST(-0600)] <athena> ick indeed

[12:39:08 CST(-0600)] <EricDalquist> it adds a request wrapper

[12:39:12 CST(-0600)] <EricDalquist> which overrides getRemoteUser()

[12:39:21 CST(-0600)] <athena> oh ugh

[12:39:21 CST(-0600)] <EricDalquist> and delegates that to the spring SecurityContextHolder

[12:39:25 CST(-0600)] <EricDalquist> yeah

[12:39:27 CST(-0600)] <EricDalquist> I'

[12:39:36 CST(-0600)] <EricDalquist> I'm not sure what to do :/

[12:39:52 CST(-0600)] <athena> yeah give me a second to try and force my brain to think about this

[12:39:59 CST(-0600)] <athena> spring-sec always gives me a headache

[12:40:15 CST(-0600)] <EricDalquist> ok

[12:40:29 CST(-0600)] <athena> how is the remote-user stuff implemented nwo?

[12:40:32 CST(-0600)] <athena> do we have a filter?

[12:40:44 CST(-0600)] <EricDalquist> no, we have RemoteUserPersonManager

[12:40:53 CST(-0600)] <EricDalquist> which is called by LoginController.service(req,res)

[12:41:09 CST(-0600)] <EricDalquist> and the RUPM calls req.getRemoteUser()

[12:41:16 CST(-0600)] <athena> must have at least a security context as well?

[12:41:54 CST(-0600)] <EricDalquist> it gets that after the getRemoteUser call

[12:42:06 CST(-0600)] <athena> ok

[12:42:32 CST(-0600)] <EricDalquist> https://github.com/Jasig/uPortal/blob/master/uportal-war/src/main/java/org/jasig/portal/security/provider/RemoteUserPersonManager.java#L71

[12:42:59 CST(-0600)] <EricDalquist> so what are we using the Spring security SecurityContextHolder for right now?

[12:43:08 CST(-0600)] <EricDalquist> I know you did some work to do partial integration

[12:43:10 CST(-0600)] <athena> it seems like the simplest thing to do would be to modify ProtalPreAuthenitcatedProcessingFilter

[12:43:29 CST(-0600)] <athena> basically we're skipping all of spring's authentication stuff and just doing it manually ourselves

[12:43:54 CST(-0600)] <athena> we have a filter that can create a spring SecurityContext based on whether the user is authenticatd or not

[12:44:16 CST(-0600)] <athena> in particular there's a block: if (loginPath.equals(currentPath)) {

[12:44:16 CST(-0600)] <athena> // clear out the current security context so we can re-establish

[12:44:16 CST(-0600)] <athena> // it once the new session is established

[12:44:16 CST(-0600)] <athena> SecurityContextHolder.clearContext();

[12:44:16 CST(-0600)] <athena> chain.doFilter(request, response);

[12:44:17 CST(-0600)] <athena> }

[12:45:48 CST(-0600)] <EricDalquist> ok'

[12:46:00 CST(-0600)] <EricDalquist> so I also see the getPreAuthenticatedPrincipal and getPreAuthenticatedCredentials methods

[12:46:05 CST(-0600)] <EricDalquist> how do those get triggered?

[12:46:16 CST(-0600)] <EricDalquist> since they have the correct person manager injected they would work

[12:46:53 CST(-0600)] <athena> yeah i don't think those should need modification

[12:46:59 CST(-0600)] <athena> but those are part fo the core spring API

[12:47:15 CST(-0600)] <EricDalquist> I guess I'm wondering if there is something I should do to trigger them

[12:47:27 CST(-0600)] <athena> i don't think so

[12:47:51 CST(-0600)] <athena> think those are used by spring's filters, the authorization framework, etc.

[12:47:57 CST(-0600)] <athena> so in that block i copied

[12:48:02 CST(-0600)] <athena> perhaps we can just set the remote user?

[12:49:06 CST(-0600)] <EricDalquist> not sure what you mean

[12:49:44 CST(-0600)] <EricDalquist> the problem I think I'm seeing is that Spring is adding a request wrapper that consults the SecurityContextHolder but getPreAuthenticatedPrincipal is never called

[12:52:44 CST(-0600)] <EricDalquist> so the problem could be that for remote_user PortalPreAuthenticatedProcessingFilter needs to call super.doFilter

[12:53:00 CST(-0600)] <EricDalquist> that looks like it might trigger the auth correctly

[12:53:33 CST(-0600)] <athena> ahh

[12:53:55 CST(-0600)] <athena> let me take a look at the base class

[12:54:11 CST(-0600)] <athena> i mean, that logic is being called at some point, otherwise i don't think any of this would work

[12:54:15 CST(-0600)] <EricDalquist> the key lines from super.doFilter is

[12:54:16 CST(-0600)] <EricDalquist> if (requiresAuthentication((HttpServletRequest) request)) {

[12:54:16 CST(-0600)] <EricDalquist> doAuthenticate((HttpServletRequest) request, (HttpServletResponse) response);

[12:54:16 CST(-0600)] <EricDalquist> }

[12:54:17 CST(-0600)] <EricDalquist> yeah

[12:54:25 CST(-0600)] <EricDalquist> and that gets called for anything other than login/logout

[12:54:51 CST(-0600)] <EricDalquist> somehow this class and LoginController need to play nice together

[12:55:01 CST(-0600)] <athena> ok.

[12:55:05 CST(-0600)] <athena> i think i see what the issue is

[12:55:11 CST(-0600)] <EricDalquist> I'm almost wondering if the logic from LoginController just needs to get moved into PortalPreAuthenticatedProcessingFilter

[12:55:23 CST(-0600)] <athena> we specifically skipped the their logic for login/logout because we actually don't have any of that information yet

[12:55:42 CST(-0600)] <athena> we want to have a person object to construct the security context with, and that's not available yet until login is called

[12:55:53 CST(-0600)] <EricDalquist> right

[12:55:54 CST(-0600)] <athena> and yes, i think the solution might be to refactor the login controller

[12:56:00 CST(-0600)] <athena> that whole class is a mess and needs refactoring anyway

[12:56:05 CST(-0600)] <EricDalquist> and then we have a catch-22 with remote_user auth

[12:56:06 CST(-0600)] <athena> we have a lot of weird manual stuff in thre

[12:56:09 CST(-0600)] <athena> yeah

[12:56:18 CST(-0600)] <EricDalquist> since we need to have the person object to make the spring req filter happy

[12:56:22 CST(-0600)] <athena> yep

[12:56:23 CST(-0600)] <EricDalquist> blarg

[12:56:48 CST(-0600)] * EricDalquist wishes he had time to just move everything to spring sec

[12:56:57 CST(-0600)] <athena> well, we're not that far off, really

[12:56:58 CST(-0600)] <EricDalquist> instead I'm trying to get this fixed locally asap

[12:57:11 CST(-0600)] <EricDalquist> just rm-Rf all the ISecurityContext bs

[12:57:24 CST(-0600)] <athena> yep.

[12:57:43 CST(-0600)] <athena> so i'd be willing to bet you could implement a really quick local fix by adding the remote user to the security context inside that login block

[12:58:27 CST(-0600)] <athena> when the filter executes, do you start out w/ the right remote user in the request?

[12:58:35 CST(-0600)] <EricDalquist> yes

[12:58:37 CST(-0600)] <athena> ok

[12:59:23 CST(-0600)] <athena> i think my suggestion would be to create an Authentication implementation that's really basic and can only be used by the login controller

[12:59:42 CST(-0600)] <athena> then call SecurityContextHolder.getContext().setAuthentication(yourThing) in that login block

[13:00:12 CST(-0600)] <athena> the login controller will overwrite that authentication object when logging in anyway

[13:00:29 CST(-0600)] <EricDalquist> uhg

[13:00:46 CST(-0600)] <EricDalquist> so I'll set it if remote_user is eet

[13:00:57 CST(-0600)] <EricDalquist> and then it will hopefully get re-set after Login runs

[13:01:37 CST(-0600)] <athena> yeah

[13:01:43 CST(-0600)] <EricDalquist> (sad)

[13:01:55 CST(-0600)] <EricDalquist> now I need to figure out a way to test remote_user auth on my local machine

[13:01:58 CST(-0600)] <athena> i guess the alternative would be to move the person stuff into the filter, but leave the auth stuff in the login controller

[13:05:34 CST(-0600)] <EricDalquist> yeah I'm going to see what happens if I call super.doFilter in the login block of PortalPreAuthenticatedProcessingFilter

[13:05:54 CST(-0600)] <EricDalquist> that will essentially let us boot-strap the IPerson in the filter

[13:06:03 CST(-0600)] <EricDalquist> and it looks like LoginController just modifies it by ref

[13:07:43 CST(-0600)] <athena> i'm not sure if that's actually going to work

[13:07:50 CST(-0600)] <athena> since it'll get back null for the principal and credentials

[13:08:17 CST(-0600)] <EricDalquist> oh right

[13:08:24 CST(-0600)] <EricDalquist> and the login servlet invalidates the session

[13:08:25 CST(-0600)] <EricDalquist> arg

[13:08:33 CST(-0600)] <athena> i think that base filter requires that you actually be authenticated, also

[13:08:47 CST(-0600)] <athena> and our current implementation returns null for those things, since we don't know anything about them yet

[13:08:56 CST(-0600)] <EricDalquist> right

[13:10:58 CST(-0600)] <athena> ok.

[13:11:09 CST(-0600)] <athena> so . . . i mean really the right thing to do in the long run is to refactor the login stuff

[13:11:19 CST(-0600)] <athena> since that'll get us closer to being able to use spring-sec more fully anyway

[13:11:29 CST(-0600)] <athena> how quickly do you need this fixed?

[13:11:41 CST(-0600)] <EricDalquist> about an hour ago

[13:11:42 CST(-0600)] <athena> i'm willing to help out by dealing w/ the login controller refactoring if that's helpful

[13:11:49 CST(-0600)] <EricDalquist> (smile)

[13:11:58 CST(-0600)] <athena> but i can't do that in negative 1 hour (smile)

[13:12:01 CST(-0600)] <EricDalquist> I'm dropping in a filter right now that will let me set remote_user

[13:12:19 CST(-0600)] <EricDalquist> once that is in place and I can test locally I can likely get a local fix today

[13:12:30 CST(-0600)] <athena> awesome

[13:12:33 CST(-0600)] <athena> that's good at least

[13:12:37 CST(-0600)] <EricDalquist> I'm thinking I'm going to move the idswapper unswap logic into a filter

[13:13:08 CST(-0600)] <EricDalquist> and then move the IPerson and secContext stuff into the preauth filter

[13:13:19 CST(-0600)] <EricDalquist> and let the login controller just to do the redirect

[13:13:53 CST(-0600)] <athena> yeah i think that should work

[13:47:52 CST(-0600)] <EricDalquist> man this security code is so WTF

[13:48:16 CST(-0600)] <EricDalquist> It would be great if we could get moved to spring-sec for 4.1

[13:49:51 CST(-0600)] <athena> we totally can

[13:49:55 CST(-0600)] <athena> we're really not that far away

[13:50:00 CST(-0600)] <EricDalquist> that would be wonderful

[13:51:01 CST(-0600)] <EricDalquist> I'll have time to help after early april

[13:51:33 CST(-0600)] <athena> awesome

[13:51:36 CST(-0600)] <athena> i don't think it'd take much

[13:51:43 CST(-0600)] <athena> impersonation

[13:51:46 CST(-0600)] <athena> a login form

[13:51:52 CST(-0600)] <athena> couple other small things

[13:51:56 CST(-0600)] <athena> and then documentation

[13:52:22 CST(-0600)] <EricDalquist> yeah

[13:52:33 CST(-0600)] <EricDalquist> and I think spring-sec has some support for impersonation already

[13:53:53 CST(-0600)] <athena> yeah

[13:53:56 CST(-0600)] <athena> unsurprising

[13:53:57 CST(-0600)] <EricDalquist> http://static.springsource.org/spring-security/site/docs/3.1.x/reference/runas.html

[13:54:09 CST(-0600)] <athena> just need to put a few pieces together and make sure it works w/ our stuff

[13:54:15 CST(-0600)] <EricDalquist> yeah

[13:54:31 CST(-0600)] <EricDalquist> there shouldn't be much code that references ISecurityContext any more

[13:54:41 CST(-0600)] <EricDalquist> the big tests will be cas proxies and clearpass

[13:55:25 CST(-0600)] <athena> oh! clearpass, yes

[13:55:29 CST(-0600)] <athena> cas proxy should be really easy

[13:55:41 CST(-0600)] <athena> we might have to create a plugin for clearpass, but that won't be much work

[13:56:02 CST(-0600)] <athena> the code is going to be so much more sane

[13:56:05 CST(-0600)] <athena> (smile)

[13:58:09 CST(-0600)] <EricDalquist> yes

[14:00:48 CST(-0600)] <EricDalquist> ok ... fix appears to work for remote user

[14:00:51 CST(-0600)] <EricDalquist> testing with local auth

[14:04:08 CST(-0600)] <EricDalquist> broke local auth ...

[14:06:02 CST(-0600)] <EricDalquist> maybe not, forgot to change config ...

[14:42:31 CST(-0600)] <drewwills> Does anyone know what it takes to get a new project in JIRA? The NotificationPortlet is supposed to have one, but neither Benito nor I seem to be able to create one or get one created

[14:44:42 CST(-0600)] <athena> i thought the incubation group was supposed to help out with steps like that

[14:44:50 CST(-0600)] <athena> not sure who all has admin privs though

[14:46:03 CST(-0600)] <drewwills> yeah Benito is on the group and the representative assigned to the incubation ticket

[14:47:05 CST(-0600)] <athena> maybe send a note to infrastructure?

[15:04:04 CST(-0600)] <holdorph> is the notification portlet going to be in github?

[15:05:56 CST(-0600)] <holdorph> because an alternative might be to use github's issue tracking system.

[15:06:47 CST(-0600)] <holdorph> i know that there are some people in the sakai community considering moving their code out of svn to github, and would like to move their issue tracking out of sakai jira to github issues.

[15:06:54 CST(-0600)] <holdorph> anyway, something to think about.

[15:09:19 CST(-0600)] <holdorph> I believe I might be able to create the jira project, so I believe athena is right, that the incubation committee should be able to take care of it. If Benito, doesn't have permissions he can talk to the rest of the incubation group and we can get it created.

[15:09:44 CST(-0600)] <holdorph> Benito was a newer member of the group, so he might not yet be in the appropriate jira permissions group or something.

[15:17:01 CST(-0600)] <EricDalquist> athena: I

[15:17:09 CST(-0600)] <EricDalquist> I'm running to to the bus

[15:17:15 CST(-0600)] <EricDalquist> but a question for you when I get home

[15:17:54 CST(-0600)] <EricDalquist> what do you think about the courses portlet getting support for multiple terms

[15:17:57 CST(-0600)] <EricDalquist> and grades

[15:18:10 CST(-0600)] <EricDalquist> we have a need to have mobile friendly grades for students by the end of the semester

[16:05:25 CST(-0600)] <EricDalquist> hello athena

[16:05:30 CST(-0600)] <athena> heya

[16:05:46 CST(-0600)] <EricDalquist> so you wrote the existing courses portlet right?

[16:05:51 CST(-0600)] <athena> yes

[16:06:14 CST(-0600)] <athena> very, very open to changes and new features

[16:06:20 CST(-0600)] <EricDalquist> I'm meeting tomorrow to show the developer who will be working on this what is out there

[16:06:22 CST(-0600)] <athena> certainly agree that things like additional semesters would be good

[16:06:31 CST(-0600)] <EricDalquist> and I'll be encouraging him to look at extending this portlet

[16:06:40 CST(-0600)] <athena> have also talked w/ awills and bruce about adding a grade calculator to it

[16:06:56 CST(-0600)] <athena> https://wiki.jasig.org/display/PLT/Courses+Portlet

[16:06:59 CST(-0600)] <athena> some screenshots there

[16:07:46 CST(-0600)] <athena> here's the current XSD: https://source.jasig.org/sandbox/CoursesPortlet/trunk/courses-portlet-api/src/main/resources/xsd/generic/course-summary.xsd

[16:07:50 CST(-0600)] <EricDalquist> yeah I think our requirements primarily are showing students their grades at the end of the semester

[16:08:14 CST(-0600)] <athena> have it bundled w/ the api and webapp separately so that LMS-side implementations can make use of JAXB serialization and such

[16:08:19 CST(-0600)] <athena> yeah, that sounds like a great start

[16:08:23 CST(-0600)] <athena> think we could support that

[16:08:24 CST(-0600)] <EricDalquist> but since this exists I'd rather have him add terms & grades features

[16:08:29 CST(-0600)] <athena> yeah

[16:08:30 CST(-0600)] <EricDalquist> great

[16:08:36 CST(-0600)] <athena> i think it should be pretty simple to add

[16:08:46 CST(-0600)] <athena> basically we need to add a notion of term and which term is current

[16:08:50 CST(-0600)] <athena> then let users select between them

[16:08:51 CST(-0600)] <EricDalquist> so he'd have to modify the UI, portlet data model and DAO API layer for terms + grades

[16:08:56 CST(-0600)] <athena> yep

[16:09:02 CST(-0600)] <EricDalquist> and then write a custom DAO impl for our SOAP services

[16:09:13 CST(-0600)] <athena> even just a select menu for semesters at the top would be an OK start (smile)

[16:09:17 CST(-0600)] <EricDalquist> yeah

[16:09:31 CST(-0600)] <athena> here's our sample XML file: https://source.jasig.org/sandbox/CoursesPortlet/trunk/courses-portlet-webapp/src/main/resources/mock-data/mock-course-summary.xml

[16:09:32 CST(-0600)] <EricDalquist> I was thinking you could do like: "<< Spring 2012 >>"

[16:09:41 CST(-0600)] <athena> some of the data isn't super structured yet - like the meeting times

[16:09:48 CST(-0600)] <athena> yeah, i think that's a good idea for an interface

[16:09:48 CST(-0600)] <EricDalquist> so that XML is for the default DAO impl?

[16:09:50 CST(-0600)] <athena> scroll through in order

[16:10:01 CST(-0600)] <athena> probably only show the semesters that users actually have courses for

[16:10:06 CST(-0600)] <athena> yeah, it is

[16:10:10 CST(-0600)] <athena> that's the mock data

[16:10:14 CST(-0600)] <EricDalquist> ok

[16:11:53 CST(-0600)] <EricDalquist> well hopefully the conversation tomorrow goes well

[16:11:56 CST(-0600)] <athena> hope so!

[16:12:02 CST(-0600)] <EricDalquist> I'll be encouraging him toward collaborating with you as well

[16:12:08 CST(-0600)] <athena> think it'd be pretty straightforward to add, and it's certainly on the project roadmap

[16:12:10 CST(-0600)] <EricDalquist> not sure if he has ever done work on an OSS project before

[16:12:15 CST(-0600)] <EricDalquist> that is moving to GitHub right?

[16:12:20 CST(-0600)] <athena> and i think i can find some time to help out even as part of the umobile effort

[16:12:21 CST(-0600)] <athena> yes

[16:12:39 CST(-0600)] <EricDalquist> ok

[16:12:56 CST(-0600)] <EricDalquist> since that will be soon I'll see if I can talk him through the git workflow as well

[16:13:03 CST(-0600)] <athena> (smile)

[16:13:04 CST(-0600)] <EricDalquist> we should have a real UW org soon

[16:13:10 CST(-0600)] <athena> awesome

[16:13:12 CST(-0600)] <EricDalquist> so we can fork the portlet there for him to work on

  • No labels