Skip to content

Added new stateless authentication that can be used in a REST api. #650

Merged
merged 15 commits into from Jul 11, 2012

3 participants

@bsommardahl

Stateless authentication was missing for me and I kept seeing other people on the web looking for the same thing. I was designing, what I hoped to be, a REST API. I think, by its nature, a REST api would be stateless. Each request should re-authenticate the caller. The existing authentication methods looked like they used cookies or session to hang on to the logged in user from request to request. The new authentication method I have added (StatelessAuthentication) provides exactly what I needed. I also tried to make it flexible enough for anyone else to implement. I patterned much after your FormsAuthentication class with its respective configurer. Cheers.

@bsommardahl

Here's how I'm using "StatelessAuthentication":

var mobileUserFetcher = container.GetInstance<IMobileUserFetcher>();
StatelessAuthentication.Enable(pipelines, new StatelessAuthenticationConfiguration(c => {
    var apiKeyFromRequest = ((DynamicDictionaryValue)c.Request.Query.ApiKey).Value 
        ?? ((DynamicDictionaryValue)c.Request.Form.ApiKey).Value;

    if (apiKeyFromRequest == null) return null;

    var mobileUser = mobileUserFetcher.Fetch(apiKeyFromRequest.ToString());
    return new ApiUserIdentity(mobileUser);
}));
@bsommardahl

Hi Skinny. What do you mean by "Contexts are last in line"? Am I adding this to the pipeline incorrectly?

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

@bsommardahl @AcklenAvenue @skinny do you guys all work together or what? Noticed some stuff where signed off by @bsommardahl is that some sort of GitHub for Enterprise feature? :) A sample application for this would be nice

@bsommardahl

Working on a sample application now. I work at Acklen Avenue, and, for some reason, all my commits come in under AcklenAvenue. It's not completely incorrect, so I haven't fought it. :)

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Needs rebasing =)

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Nevermind..github was playing tricks on me!

@bsommardahl

So, how does it look? Does this fit in Nancy's future?

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Will have a look at this tonight (btw right now github is telling me it cannot be automatically merged)

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

So just to make sure I get it,,. the idea is to have an auth method that does not use cookies etc to remember you on round-trips, but instead you pass in your credentials each time? Say like an oauth key

@bsommardahl

That's the idea. I'm trying to be true to REST that is supposed to be stateless:

"The client–server communication is further constrained by no client context being stored on the server between requests. Each request from any client contains all of the information necessary to service the request, and any session state is held in the client. The server can be stateful; this constraint merely requires that server-side state be addressable by URL as a resource. This not only makes servers more visible for monitoring, but also makes them more reliable in the face of partial network failures as well as further enhancing their scalability." - from http://en.wikipedia.org/wiki/Representational_State_Transfer

So, the way I read that in the context of login and auth, the client should be passing in some type of authentication package with each request.

@bsommardahl

Does github have any feedback on why it can't be merged automatically. I'll fix whatever I need to on my end.

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

I believe it just needs to be rebased. I have an idea for this pull request that I'll let iterate in my head until tomorrow morning..will post it then if i still think it makes sense =D

@bsommardahl bsommardahl reopened this Jun 28, 2012
@bsommardahl

Great! In the mean time, I'll get it rebased.

@thecodejunkie thecodejunkie and 1 other commented on an outdated diff Jun 29, 2012
...ation.Stateless/Nancy.Authentication.Stateless.csproj
+ <DefineConstants>TRACE</DefineConstants>
+ <ErrorReport>prompt</ErrorReport>
+ <WarningLevel>4</WarningLevel>
+ </PropertyGroup>
+ <ItemGroup>
+ <Reference Include="System" />
+ <Reference Include="System.Core" />
+ <Reference Include="System.Xml.Linq" />
+ <Reference Include="System.Data.DataSetExtensions" />
+ <Reference Include="Microsoft.CSharp" />
+ <Reference Include="System.Data" />
+ <Reference Include="System.Xml" />
+ </ItemGroup>
+ <ItemGroup>
+ <Compile Include="StatelessAuthentication.cs" />
+ <Compile Include="Properties\AssemblyInfo.cs" />
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

AssemblyInfo.cs is blocked in the .gitignore file. You should link in the src/SharedAssemblyInfo.cs file and put it in the properties folder

@bsommardahl
bsommardahl added a note Jun 30, 2012

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie and 1 other commented on an outdated diff Jun 29, 2012
...y.Authentication.Stateless/StatelessAuthentication.cs
@@ -0,0 +1,63 @@
+namespace Nancy.Authentication.Stateless
+{
+ using System;
+ using Nancy.Bootstrapper;
+
+ /// <summary>
+ /// Nancy forms authentication implementation
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

forms -> stateless

@bsommardahl
bsommardahl added a note Jun 30, 2012

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on the diff Jun 29, 2012
...ation.Stateless/Nancy.Authentication.Stateless.csproj
@@ -0,0 +1,61 @@
+<?xml version="1.0" encoding="utf-8"?>
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Just a general thought, but is the project configured to be built for the MonoDebug and MonoRelease build configurations?

@bsommardahl
bsommardahl added a note Jun 30, 2012

I just build using MonoDebug and MonoRelease. No errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie and 1 other commented on an outdated diff Jun 29, 2012
...y.Authentication.Stateless/StatelessAuthentication.cs
+ throw new ArgumentNullException("configuration");
+ }
+
+ if (!configuration.IsValid)
+ {
+ throw new ArgumentException("Configuration is invalid", "configuration");
+ }
+
+ pipelines.BeforeRequest.AddItemToStartOfPipeline(GetLoadAuthenticationHook(configuration));
+ }
+
+ /// <summary>
+ /// Gets the pre request hook for loading the authenticated user's details
+ /// from apikey given in request.
+ /// </summary>
+ /// <param name="configuration">Forms authentication configuration to use</param>
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Forms -> Stateless

@bsommardahl
bsommardahl added a note Jun 30, 2012

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie and 1 other commented on an outdated diff Jun 29, 2012
...ion.Stateless/StatelessAuthenticationConfiguration.cs
@@ -0,0 +1,34 @@
+namespace Nancy.Authentication.Stateless
+{
+ using System;
+ using Nancy.Security;
+
+ /// <summary>
+ /// Configuration options for stateless authentication
+ /// </summary>
+ public class StatelessAuthenticationConfiguration
+ {
+ internal Func<NancyContext, IUserIdentity> GetUserIdentity;
+
+ public StatelessAuthenticationConfiguration(Func<NancyContext, IUserIdentity> getUserIdentity)
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

XML comments are missing for the ctor

@bsommardahl
bsommardahl added a note Jun 30, 2012

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on the diff Jun 29, 2012
....Stateless/Nancy.Demo.Authentication.Stateless.csproj
@@ -0,0 +1,69 @@
+<?xml version="1.0" encoding="utf-8"?>
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Same here, will it build for the mono build configurations?

@bsommardahl
bsommardahl added a note Jun 30, 2012

Believe so. Worked for my environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie commented on the diff Jun 29, 2012
...ion.Stateless/StatelessAuthenticationConfiguration.cs
@@ -0,0 +1,34 @@
+namespace Nancy.Authentication.Stateless
+{
+ using System;
+ using Nancy.Security;
+
+ /// <summary>
+ /// Configuration options for stateless authentication
+ /// </summary>
+ public class StatelessAuthenticationConfiguration
+ {
+ internal Func<NancyContext, IUserIdentity> GetUserIdentity;
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Why internal? Private readonly should do the trick right?

@bsommardahl
bsommardahl added a note Jun 30, 2012

That Func is used in the GetLoadAuthenticationHook method of the StatelessAuthentication class. I used internal instead of public just to keep the API cleaner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie thecodejunkie and 1 other commented on an outdated diff Jun 29, 2012
....Stateless/Nancy.Demo.Authentication.Stateless.csproj
+ </PropertyGroup>
+ <ItemGroup>
+ <Reference Include="System" />
+ <Reference Include="System.Core" />
+ <Reference Include="System.Xml.Linq" />
+ <Reference Include="System.Data.DataSetExtensions" />
+ <Reference Include="Microsoft.CSharp" />
+ <Reference Include="System.Data" />
+ <Reference Include="System.Xml" />
+ </ItemGroup>
+ <ItemGroup>
+ <Compile Include="DemoUserIdentity.cs" />
+ <Compile Include="Models\UserModel.cs" />
+ <Compile Include="StatelessAuthBootstrapper.cs" />
+ <Compile Include="MainModule.cs" />
+ <Compile Include="Properties\AssemblyInfo.cs" />
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Same here for the AssemblyInfo -> SharedAssemblyInfo files

@bsommardahl
bsommardahl added a note Jun 30, 2012

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

All in all I really like this one and we'll definitely be pulling it into Nancy once the review comment have been sorted out. It's going to be a handy thing to have for my OAuth branch :)

So the idea I got late last night you already had implemented! It was to use a Func to get the key out. Nicely done!

So a part from the inline review comments, there are two general comments too

  • All using statements should be placed inside the namespace declaration to be inline with the convention we use in Nancy
  • There is no nuspec file for it, could you add it so we get a nuget out of it?

Thanks!

@bsommardahl

I'll work on all these over the weekend.

@bsommardahl

How's the branch looking right now? I think I've made all the changes you were looking for.

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

If you could rebase it, then I'll pull it in straight away =)

@bsommardahl

I ran 'git rebase StatelessAuthentication' and it says 'Current branch StatelessAuthentication is up to date.' I may be doing it wrong. What's the syntax I should use?

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

ah. so what you do is checkout your master and pull in ours to that.. then you move over to your stateless branch again and do git rebase master and resolve any merge conflicts that might happen have a read here on how to manage changes from upstream https://github.com/NancyFx/Nancy/wiki/Git-Workflow

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

By the way upstream is just an alias for our repo. Upstream is normally used as a name for the main repo. To add it you do git remote add upstream <github url to our repo>

@bsommardahl

Not sure the rebase worked. I just ran it again to see if it would say that everything was up-to-date, but it's having me run the whole rebase again. Honestly, the is my first rebase... always wanted to do it but never had a reason. I'm working through the rebase again, right now.

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
@bsommardahl

You're right about that. There's always a point at which I'm forced to learn something I've been putting off. :)

I believe it's rebased correctly this time. I think I need to rebase more often based on that experience.

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Couple of things to fix

  • Nancy.Demo.Authentication.Stateless seems to think it's a class library, not a web project
  • The added nuspec is called nancy.authentication.forms.nuspec so forms should be changed to stateless. Copy and paste for the win ;)
  • Looking at your commits, it does look like the rebase didn't go as expected, because all of your commits are there in duplicates, each with unique SHA1's .. don't think it's a big deal in this case, but it's good to know for future rebases
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Any updates on this?

@bsommardahl

Sorry, my github notifications were going to spam and I got busy with something for a client. I'll fix the things you mentioned today. I'll keep in mind my experience with rebase.. hopefully it'll go better the second time around. :)

@bsommardahl

I'm still working on this.

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Is it the rebase that's the "problem" ?

@bsommardahl

No, I was reworking the demo. I had not put enough effort into the demo. Now, It's much better. It demonstrates a stand-alone rest api and a separate website that hits it.

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

You've left a backup folder in src\Backup not sure, but it looks like some of the files in there might be referenced too

@bsommardahl

That's fixed.

@thecodejunkie thecodejunkie merged commit b1e09ac into NancyFx:master Jul 11, 2012
@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

There were one tiny thing left to fix, and that was replacing the AssemblyInfo.cs references, in the demo projects, with the SharedAssemblyInfo.cs file instead. We block the AssemblyInfo.cs file in our .gitignore file. Pulled it in and sorted out locally.

Thank you for your contribution!

@thecodejunkie
Custodians of the Super-Duper-Happy-Path member

Oh, just noticed you had left a Nancy.Demo.Authentication.Stateless_old folder in there too. zzzaap killed it!

@bsommardahl
@daveaglick

This is an excellent addition - I was looking for this exact capability. Thanks to those involved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.