These are chat archives for spring-guides/tut-spring-security-and-angular-js

9th
Jan 2015
Thorsten Späth
@tspaeth
Jan 09 2015 09:52
np - but yes, you'Re right. It doesn't at the end really matter, how and what the HTTP-header is called that is sent to the REST-Server from any (arbitrary) client - be it JSESSIONID or X-AUTH-HEADER or X-FOO-BAR.
if it is not toooooo late, I'll continue the review today afternoon as I'm still stuck in my normal project work doing alot of code reviews and bugfixes... so concentration is currently a bit out of order :-)
Dave Syer
@dsyer
Jan 09 2015 09:53
@rwinch is reviewing today as well
I plan to publish parts I&II early next week
Rob Winch
@rwinch
Jan 09 2015 15:20
It may be nice in https://github.com/dsyer/spring-security-angular/tree/master/basic#create-a-new-project to tell the user they have a few options on how to create the new project and provide bullets that link to the subsections This helps set the readers expectations that the subsections are an OR rather than an AND Otherwise a new Boot user doesn't realize it is an OR until they get to the Boot CLI section
Below each option you might add a sentence about, you could now skip to "Add a Home Page". This makes it easier for someone who just wants to start from scratch skip stuff they don't need to read
Dave Syer
@dsyer
Jan 09 2015 15:23
Sounds good
In the other blogs that need a new project I just use the curl example and link back to part I
Rob Winch
@rwinch
Jan 09 2015 15:25
I think that is good...it is a series so you can hopefully assume they read the first post
Dave Syer
@dsyer
Jan 09 2015 15:25
I'm not sure how the UX will work out with links, but I'll try it
Rob Winch
@rwinch
Jan 09 2015 15:27
https://github.com/dsyer/spring-security-angular/tree/master/basic#using-curl directions also extract the contents in the current directory...
If that happened to me by blindly following directions it may be a bit messy
perhaps tell them to create a new directory first or add an argument to specify a new directory to put it in
Rob Winch
@rwinch
Jan 09 2015 15:35
That last comment is actually true of all of the mechanisms for creating a new project
Any reason it doesn't contain the project in a new folder?
Dave Syer
@dsyer
Jan 09 2015 15:37
What is "it"?
The article?
Hmm. Github strips the anchors out of my source
It has the navigation changes you suggested but the links are broken. Annoying
Rob Winch
@rwinch
Jan 09 2015 15:41
:(
Why doesn't initilizr create a new folder for the compressed project
btw the :( was at GitHub anchors being broken
Dave Syer
@dsyer
Jan 09 2015 15:42
It does if you add a location argument
Rob Winch
@rwinch
Jan 09 2015 15:42
ic....perhaps that would be valuable for the blog
Dave Syer
@dsyer
Jan 09 2015 15:42
Look now
Is that valuable?
Rob Winch
@rwinch
Jan 09 2015 15:43
what about the website and the CURL?
Dave Syer
@dsyer
Jan 09 2015 15:43
The anchors work now too BTW (but only by hacking them to be the same as githubs)
Rob Winch
@rwinch
Jan 09 2015 15:43
oh name is ui
is that what fixes it for Curl?
Dave Syer
@dsyer
Jan 09 2015 15:43
curl has a "mkdir" now
Rob Winch
@rwinch
Jan 09 2015 15:43
oh oops
Dave Syer
@dsyer
Jan 09 2015 15:44
The website one doesn't say
Rob Winch
@rwinch
Jan 09 2015 15:44
Sorry I missed that
if you like you can do...
$ mkdir ui && cd $_
Dave Syer
@dsyer
Jan 09 2015 15:44
You get a .zip file fro mthe home page. You can do whatever you want with it.
$_ works too but is it bash-specific?
Rob Winch
@rwinch
Jan 09 2015 15:44
good point
sorry I always think in bash
I guess an improvement for the web UI (at least IMHO) would be to have the zip file have a folder
what I would do is download it and uncompress it to my ~/code/ folder
Dave Syer
@dsyer
Jan 09 2015 15:46
Not really relevant here
Rob Winch
@rwinch
Jan 09 2015 15:46
possibly not except if I follow the instructions that problem happens to me
Dave Syer
@dsyer
Jan 09 2015 15:47
That's a feature request to spring-io/initializr
Rob Winch
@rwinch
Jan 09 2015 15:47
If you like I can log a point to the website
ok...either I will do that Either way it impacts the reader of the blog is all I was pointing out
Dave Syer
@dsyer
Jan 09 2015 15:50
Updated initializr instructions
Rob Winch
@rwinch
Jan 09 2015 15:52
spring-io/initializr#65
Dave Syer
@dsyer
Jan 09 2015 15:53
Ta
Rob Winch
@rwinch
Jan 09 2015 15:53
I'll be having a brief interruption in my workday shortly w/ a repair man coming
Ta ?
Dave Syer
@dsyer
Jan 09 2015 15:53
British slang for "thanks"
Rob Winch
@rwinch
Jan 09 2015 15:53
Ahh
Dave Syer
@dsyer
Jan 09 2015 15:54
I think it will be auto linked by the renderer
But I can make it explicit
Rob Winch
@rwinch
Jan 09 2015 15:55
Gotcha...wasn't happening in the README is why I mentioned it
Dave Syer
@dsyer
Jan 09 2015 15:55
Hmm weird
I thought github did that
Rob Winch
@rwinch
Jan 09 2015 15:55
maybe not for localhost for security reasons?
Dave Syer
@dsyer
Jan 09 2015 15:56
Maybe
Rob Winch
@rwinch
Jan 09 2015 15:56
I know StackOverflow won't allow localhost links
Dave Syer
@dsyer
Jan 09 2015 15:56
I made it explicit anyway
Rob Winch
@rwinch
Jan 09 2015 15:57
Very minor, but may be nice to have a context of where <build> goes into a pom entered into the snippet
It does say pom.xml in the reading above, but personally I really like a label just above any file that is being edited (i.e. bold pom.xml just above the code would be nice)
Very much style comments so feel free to ignore if you disagree or don't care
Dave Syer
@dsyer
Jan 09 2015 15:58
asciidoc is nice for that
How do you do it in markdown?
Rob Winch
@rwinch
Jan 09 2015 15:59
I don't know an official way but I would just do a bold lettering above it
Dave Syer
@dsyer
Jan 09 2015 16:00
I don't like the way that gets formatted
Rob Winch
@rwinch
Jan 09 2015 16:00
fair enough
feel free to disregard
Another really minor thing end of https://github.com/dsyer/spring-security-angular/tree/master/basic#wro4j-source-files might be nice to have another localhost:8080 link for convenience (i.e. if they closed their browser makes it easier on the reader...no need to type, remember the port, context root, etc)
Dave Syer
@dsyer
Jan 09 2015 16:07
Not here
Github again
Try now
Rob Winch
@rwinch
Jan 09 2015 16:08
Maybe if you add the code fence?
fixed
Dave Syer
@dsyer
Jan 09 2015 16:08
You can't use code fences to put source in lists
Rob Winch
@rwinch
Jan 09 2015 16:08
oh lovely
I forget all these fun things about markdown
ok my handyman is here...brb
Dave Syer
@dsyer
Jan 09 2015 16:09
Thanks
but I have DemoApplication
might be nice to have a path specified for where to look for it too
Dave Syer
@dsyer
Jan 09 2015 16:21
You have DemoApplication because you didn't use the curl command
Rob Winch
@rwinch
Jan 09 2015 16:21
Yes I ended up w/ the cmd line app
either way it is a bit confusing
as always feel free to ignore if you disagree
Dave Syer
@dsyer
Jan 09 2015 16:22
It's a tough call
Rob Winch
@rwinch
Jan 09 2015 16:23
The Annotations at the top change too
Dave Syer
@dsyer
Jan 09 2015 16:23
When you get to the point that there are multiple main() methods in an article you have to give them names (not Demo)
Rob Winch
@rwinch
Jan 09 2015 16:23
(i.e. I have the following...
@Configuration
@ComponentScan
@EnableAutoConfiguration
It uses the SpringBootApplication annotation (which is the same but new users won't know that)
Dave Syer
@dsyer
Jan 09 2015 16:23
Right. That's to support Boot 1.x.
Rob Winch
@rwinch
Jan 09 2015 16:23
also it adds RestController annotation
I guess my point is that a reader might miss RestController annotation
and be confused why the three annotations are now SpringBootApplication (or they might not know that they are even related)
I'll just make that a general comment path and file name for any code snippet would be nice above any code
Dave Syer
@dsyer
Jan 09 2015 16:26
Read it again now.
Better?
Rob Winch
@rwinch
Jan 09 2015 16:26
That way if you decided to ignore I don't get annoying
Dave Syer
@dsyer
Jan 09 2015 16:27
I think that was the only file I didn't give a path to (in this blog anyway)
Rob Winch
@rwinch
Jan 09 2015 16:27
Much better...might use a NOTE: for the depending and create a link to SpringBootApplication so they can read more
Dave Syer
@dsyer
Jan 09 2015 16:27
Nice
Rob Winch
@rwinch
Jan 09 2015 16:31
Since the imports change for https://github.com/dsyer/spring-security-angular/tree/master/basic#adding-dynamic-content might be nice to include them too
For the end blog it might be nice to have a link in https://github.com/dsyer/spring-security-angular/tree/master/basic#whats-wrong-with-that to the next blog
you might be saving that if you are posting them spread out
but just pointing it out in case
Dave Syer
@dsyer
Jan 09 2015 16:35
I was going to add the "forward" links as the blogs go out
Rob Winch
@rwinch
Jan 09 2015 16:36
Sounds good...and makes sense
Dave Syer
@dsyer
Jan 09 2015 16:36
I put the reverse ones in because they should be valid as soon as they are published
Rob Winch
@rwinch
Jan 09 2015 16:36
gotcha..wasn't sure if it was going out all at once or in phases
so which is blog II?
oh nm
Dave Syer
@dsyer
Jan 09 2015 16:38
"single"
Rob Winch
@rwinch
Jan 09 2015 16:38
Sorry I see the outline in the main readme now
I'm a bit slow it appears
Rob Winch
@rwinch
Jan 09 2015 16:48
So once I get to this place...
It is pretty clear that code is omitted
puts
.controller('navigation', funciton() {})
at the end
this way it is leave the top of the file alone sort of thing
Otherwise it takes a decent amount of thought to figure it out
(particularly for a JS noob like me)
Dave Syer
@dsyer
Jan 09 2015 16:50
You mean it's better to out the "omitted code" comment at the bottom?
instead of the top?
Rob Winch
@rwinch
Jan 09 2015 16:50
better than in the middle
At least as I am reading now...(could be wrong)... it appears the first section of code has the navigation route in the middle of the file
the second code snippet seems to assume you remove the navigation route and then add it again but to the end of the file
it would be easier for me as a reader to say change the end of the file to have this updated navigation route
Dave Syer
@dsyer
Jan 09 2015 16:51
Better now?
Rob Winch
@rwinch
Jan 09 2015 16:51
It is quite likely I have read wrong
Dave Syer
@dsyer
Jan 09 2015 16:52
No you are right. I just didn't get you at first.
Rob Winch
@rwinch
Jan 09 2015 16:52
Yes much better
(i.e. Principal)
(if you like I can stop providing that feedback just let me know)
Dave Syer
@dsyer
Jan 09 2015 16:55
I don't like adding imports to Java source
I figure anyone can do that in an IDE
Rob Winch
@rwinch
Jan 09 2015 16:55
If I'm not familiar w/ boot add some configuration to our application isn't clear for https://github.com/dsyer/spring-security-angular/tree/master/single#handling-the-login-request-on-the-server
ok I will stop commenting that then
I tend to disagree though...there are a lot of people that do not know how to do that
I've found out first hand through publications
Dave Syer
@dsyer
Jan 09 2015 16:56
I'll think about it
Rob Winch
@rwinch
Jan 09 2015 16:56
no worries
I'll stop adding that comment
Dave Syer
@dsyer
Jan 09 2015 16:56
What was that about the login form config?
Rob Winch
@rwinch
Jan 09 2015 16:56
so It isn't that clear where to place it
unless you know Spring Boot
I'd mention the parent file and a context
I think many people that use Boot are new to Java
that is the power of boot...so we need to consider that they might barely know java
Before if I said the person reading your Spring article might not know Java I'd probably have laughed and said good luck figuring it out it is a pre-req
Dave Syer
@dsyer
Jan 09 2015 16:58
Ah
Rob Winch
@rwinch
Jan 09 2015 16:58
but with boot...I think things have changed....it is so easy that some may be using it that do need hand holding
it makes it much easier to understand what is happening...plus it helps when getting stacktraces (if you get a stack on this line:
.antMatchers("/*/.html", "/").permitAll().anyRequest()
you don't know where the NPE happened
(or whatever exception you got)
Dave Syer
@dsyer
Jan 09 2015 17:01
      http
        .formLogin().and()
        .authorizeRequests()
          .antMatchers("/**/*.html", "/").permitAll()
          .anyRequest().authenticated();
Like that?
Rob Winch
@rwinch
Jan 09 2015 17:02
Much better
Dave Syer
@dsyer
Jan 09 2015 17:03
I can change the Csrf cookie path if you know the incantation
How do you find the context path from the request?
I re-formatted the other http.* bits as well (look again now).
Dave Syer
@dsyer
Jan 09 2015 17:09
Also fixed a bug in the filter definition (class declaration was not valid)
Rob Winch
@rwinch
Jan 09 2015 17:13
So another issue I ran into was it seemed that hello.js was getting cached and I had to clear the cache to get it to reload
it might be nice to have a note to force a reload with SHIFT+CTRL+R or something
Dave Syer
@dsyer
Jan 09 2015 17:16
Odd. It always works for me
Maybe because I use developer tools (F12) (which I did suggest at the end somewhere)
Rob Winch
@rwinch
Jan 09 2015 17:16
perhaps user error, but didn't work until i did a forced refresh of the page
Possibly
either way feedback is yours to do as you want
Dave Syer
@dsyer
Jan 09 2015 17:19
Thanks
Rob Winch
@rwinch
Jan 09 2015 17:19

public class CsrfHeaderFilter extends OncePerRequestFilter() {

Did you intend to have the () in there? (I did a refresh since your comment about fixing it)

Dave Syer
@dsyer
Jan 09 2015 17:22
I think that's what I fixed
public class CsrfHeaderFilter extends OncePerRequestFilter() {
is what I see
Ah wrong.
Try again now
Rob Winch
@rwinch
Jan 09 2015 17:23
Still need to remove the ; } at the end I think
(i.e. no need for ; and should remove the last } )
The first security configuration for https://github.com/dsyer/spring-security-angular/tree/master/single#csrf-protection the .csrf() is unnecessary at this point

.csrf().addFilterAfter(new CsrfHeaderFilter(), CsrfFilter.class);

can be

addFilterAfter(new CsrfHeaderFilter(), CsrfFilter.class);

Dave Syer
@dsyer
Jan 09 2015 17:27
Well spotted
Fixed
Rob Winch
@rwinch
Jan 09 2015 17:29
Looks good
The other thing I'd point out is extremely minor but...
The code in the next snippet
http
.formLogin().and()
...
.csrf().csrfTokenRepository(csrfTokenRepository());
the ... and .csrf() should align with .formLogin()
because they don't modify .formLogin()
Dave Syer
@dsyer
Jan 09 2015 17:31
OK
Dave Syer
@dsyer
Jan 09 2015 17:37
I'm a bit hazy on what all those builders do
It's mostly copy-paste hit-and-hope
Rob Winch
@rwinch
Jan 09 2015 17:47
np
conceptually you can see what the value is that is returned
if the type is http it should indent one from http
similar to XML actually
sorry for the delays there I was paying the handy man
Dave Syer
@dsyer
Jan 09 2015 17:49
No worries . We must be nearly done.
I'm knocking off now. Still on phone if you have more comments.
Rob Winch
@rwinch
Jan 09 2015 17:50

From the blog https://github.com/dsyer/spring-security-angular/tree/master/single#logout it isn't clear where

$scope.logout = function() {
$http.post('logout', {}).success(function() {
$rootScope.authenticated = false;
$location.path("/");
}).error(function(data) {
$rootScope.authenticated = false;
});
}

goes

I'll just leave them here and you can do them when you get a chance

This

@Override
protected void configure(HttpSecurity http) throws Exception {
http
.formLogin().and()
.logout()
...
;
}

is missing and .and()

.logout().and()

Technically .logout() isn't even necessary because it is included by the WebSecurityConfigurerAdapter unless false is passed into the constructor

Dave Syer
@dsyer
Jan 09 2015 17:51
It says "in the navigation controller".
Rob Winch
@rwinch
Jan 09 2015 17:51
where in the navogation controller
Dave Syer
@dsyer
Jan 09 2015 17:51
I can make that more obvious I guess
Rob Winch
@rwinch
Jan 09 2015 17:51
at the end?
again JS noob
Dave Syer
@dsyer
Jan 09 2015 17:52
Sure. It's just a statement. I don't think it matters.
Rob Winch
@rwinch
Jan 09 2015 17:56

so it looks like it must be in the

function($rootScope, $scope, $http, $location, $route) {

takeaway is that would be good for some context on that
Rob Winch
@rwinch
Jan 09 2015 18:03

THIS POINT IS RATHER IMPORTANT SO PUTTING THIS HERE TO DRAW ATTENTION TO IT

The config below is dangerous in Spring MVC

.antMatchers("/*/.html", "/").permitAll()

This is dangerous because Spring MVC ignores file extensions, so they can bypass security constraints

For example:

curl -v localhost:8080/user.html

gives a 200

Obviously you don't see a username in the result because it is null
Rob Winch
@rwinch
Jan 09 2015 18:10

https://github.com/dsyer/spring-security-angular/tree/master/single#how-does-it-work

Note: it is not adequate for CSRF protection to rely on a cookie being sent back to the server because cookies can be tampered with. Since in our application the CSRF token is sent to the client as a cookie we will see it being sent back automatically by the browser, but it is the header that provides the protection.

The reason the cookie isn't safe is because the browser automatically send the cookie even if you are on another domain. The header is not automatically sent, so that is why it protects against CSRF
ANother point is you don't want JS to access your session id because then you open yourself up to XSS attacks accessing the session id
you want the session id to be HTTP Only
obviously XSS is always an issue anyways and can still act on the user's behalf but the session id is considered more valuable since you can use it outside of the browser w/out a user doing anything
Rob Winch
@rwinch
Jan 09 2015 18:20

From https://github.com/dsyer/spring-security-angular/tree/master/vanilla#client-side-changes you might change

angular.module('hello', [ 'ngRoute' ])
...
.controller('home', function($scope, $http) {
$http.get('/resource/').success(function(data) {
$scope.greeting = data;
})
});

to indicate there is content below

angular.module('hello', [ 'ngRoute' ])
...
.controller('home', function($scope, $http) {
$http.get('/resource/').success(function(data) {
$scope.greeting = data;
})
})
...
;

same with the snippet right below that shows the updates
Dave Syer
@dsyer
Jan 09 2015 18:26
roger
Rob Winch
@rwinch
Jan 09 2015 18:28
https://github.com/dsyer/spring-security-angular/tree/master/vanilla#server-side-changes describes it should load on port 9000 but it didn't change the port from 8080
Also switching between Java and Groovy seems a bit odd to me
http://localhost:9000 could be explicit link
Rob Winch
@rwinch
Jan 09 2015 18:34
If stick w/ groovy go groovy all the way in https://github.com/dsyer/spring-security-angular/tree/master/vanilla#cors-negotation CorsFilter
public can be removed from the class and the methods
also remove throws IOException, ServletException
Thorsten Späth
@tspaeth
Jan 09 2015 18:38
phew... lot to read later :-)
Rob Winch
@rwinch
Jan 09 2015 18:58

https://github.com/dsyer/spring-security-angular/tree/master/vanilla#securing-the-resource-server States:

That might not even be a problem if your network architecture mirrors the application architecture (you can just make the resource server physically inaccessible to anyone but the UI server). As a simple demonstration of that we can make the resource server only accessible on localhost.

However, you cannot restrict access to the UI server because the requests are made from the browser (not the UI server). The browser is untrusted, so the resource server cannot be protected by networking.

Rob Winch
@rwinch
Jan 09 2015 19:13
https://github.com/dsyer/spring-security-angular/blob/master/vanilla/README.md#spring-session uses RC1 but does not update maven repos...likely better to just update to .RELEASE (I think you had that planned already but just pointing this out)
Dave Syer
@dsyer
Jan 09 2015 19:15
Good point. I actually updated the source code but not the article
duh! on the network security
It applies to the next blog
But not this one
Good catch
Rob Winch
@rwinch
Jan 09 2015 19:18
https://github.com/dsyer/spring-security-angular/tree/master/vanilla#spring-session You can add the Spring Session Filter with @EnableRedisHttpSession (that should be all you need)
Dave Syer
@dsyer
Jan 09 2015 19:19
Ah. New feature.
Does it just look for a connection factory bean?
Rob Winch
@rwinch
Jan 09 2015 19:19
Possibly I think it was added in RC1 maybe
Yes
If you expose HeaderHttpSessionStrategy as a bean it will also automatically apply it
NOTE: You will want a newer Redis Server version
as it requires to support redis keyspace notifications
this is so we can send SessionDestroyedEvents (critical if using websockets so that the websocket connections associated with the httpsession can be closed)
ok another break on my side...I'm going to grab some lunch
Rob Winch
@rwinch
Jan 09 2015 20:23
For the RedisServer another option might be to use redis.embedded:embedded-redis:0.2 as is done in Spring Session samples...see https://github.com/spring-projects/spring-session/blob/1.0.0.RELEASE/samples/boot/src/main/java/sample/config/EmbeddedRedisConfiguration.java
Any reason you choose in https://github.com/dsyer/spring-security-angular/blob/master/vanilla/README.md#authentication-in-the-resource-server to use a custom header name? It could reduce the configuration some to just use x-auth-token
Rob Winch
@rwinch
Jan 09 2015 21:30
https://github.com/dsyer/spring-security-angular/blob/master/vanilla/resource/pom.xml#L39 demo.Application does not exist...it should be demo.ResourceApplication
CorsFilter should not continue the filter chain when OPTIONS is received...it needs to send an HTTP Status code of 200 if it continues the filter chain Spring Security will set the status to 401 and it will fail
Needs corrected in src and in the README.md
Even with those fixes...the resources application appears to be using NullSecurityContextRepository...it appears boot thinks its stateless or something
Rob Winch
@rwinch
Jan 09 2015 21:46
@dsyer I feel like I have hit enough issues at this point that I'll wait until they are addressed to get back to them
Dave Syer
@dsyer
Jan 09 2015 21:50
Thanks. I'll get on it Monday.
Rob Winch
@rwinch
Jan 09 2015 21:57
@dsyer No problem...have a good weekend
Rob Winch
@rwinch
Jan 09 2015 22:45
I couldn't leave it be... needed to have security.sessions=NEVER (never create an HttpSession, but use one if one exists) otherwise ApplicationWebSecurityConfigurerAdapter sets the application to stateless
after those changes it seems to work
Rob Winch
@rwinch
Jan 09 2015 22:55
after reading the next blog i realized that vanilla is a halfway point of the blog rather than the complete blog
so vanilla doesn't need anything but https://github.com/dsyer/spring-security-angular/blob/master/vanilla/resource/pom.xml#L39 demo.Application does not exist...it should be demo.ResourceApplication
The README needs all the updates mentioned though (i.e. CorsFilter changes, should document about security.sessions=NEVER, @Order and should be pointed out https://github.com/dsyer/spring-security-angular/tree/master/vanilla#authentication-in-the-resource-server )