apereo / dotnet-cas-client

Apereo .NET CAS Client
Apache License 2.0
231 stars 176 forks source link

RFC: Distributed cache provider for proxy and service ticket managers #61

Open phantomtypist opened 7 years ago

phantomtypist commented 7 years ago

The current implementations provided are only store the proxy and service tickets in-memory or on disk locally to the web application. The limitation of this model is that it will not support clustered, load balanced, or round-robin style configurations.

I plan on writing one, or several, distributed cache provider(s) for the proxy and service ticket managers. I'd like to hear back from the community what types of distributed cache systems you would be using in conjunction with the .NET CAS Client (e.g. Redis, Memcached, etc.)

The providers would each be in separate C# projects and therefore separately installable NuGet packages (e.g. DotNetCasClient.CacheProvider.Redis)

So if you want to use the Redis provider, you would:

  1. Install the DotNetCasClient NuGet package like you normally would.
  2. Install the proxy/service ticket manager cache provider you want (e.g DotNetCasClient.CacheProvider.Redis)
  3. In your web.config file, in the casClientConfig section, change the proxyTicketManager and serviceTicketManager values to RedisCacheProxyTicketManager and RedisCacheServiceTicketManager.

Note: Step 3 can actually be eliminated altogether by using web.config transformations in the cache provider NuGet package (i.e. when it's installed it'll change the values for you and when you uninstall it'll change them back to the default in-memory managers.)

Proposed Distributed Cache Providers for Proxy and Service Ticket Managers

phantomtypist commented 7 years ago

I'd also like to know what .NET framework versions you would be targeting.

sunnymoon commented 7 years ago

The issue with redis/memcached/apache ignite.net and others is actually the (non existent) support for .net 2 or 3. We, for instance have that problem (not for much longer) in production because of some legacy .net apps.

phantomtypist commented 7 years ago

Support for Memcached using the Enyim C# Memcached client the implemented in pull requests #39 and #38.

phantomtypist commented 7 years ago

@sunnymoon my plan is to support whatever we can.

Starting in version 1.1.0 of this project, we now support multiple target frameworks in the .NET CAS Client. Any of these distributed caching providers that are included in the repo will be built as separate NuGet packages that you can "add-on" to the main core nuget package to extend its functionality.

Let's say there are only Redis libraries out there that only support .NET 4.x. Then the Redis Cache Service/Proxy Ticket Manager NuGet package will only be installable on projects targeting .NET 4.x. There just won't be any support for it on the .NET 2/3.x projects you target.

The good news is it looks like pull requests #38 and #39 use a C# Memcached client (see my immediate previous comment) that supports .NET 3.x or higher. So that means that projects that target .NET 3.x/4.x could be able to use at least Memcached for distributed ticket managers.

phantomtypist commented 7 years ago

To clarify, if you open the 1.1.0 version of this projects' nuget package, you'll see that there are four distinct assemblies in it, each compiled for a different target framework. This will allow us to continue to support older framework versions while at the same time adding new features for modern framework versions.

TheHokieCoder commented 7 years ago

Does anyone have any recommendations on how to store the CasAuthenticationTicket object in a service ticket manager that is implemented using SQL Server? Since CasAuthenticationTicket and Assertion are both serializable, its not a problem. But I have seen some recommendations for using a StreamWriter to serialize the object and save it in a VARBINARY column and some for using a JSON framework to serialize the object and save it in a NVARCHAR column. I personally think the JSON method would be nice because it would be human-readable in the database for easier debugging. Any other options?

TheHokieCoder commented 6 years ago

FWIW I am currently working on an implementation of a service ticket manager that uses MS SQL Server for the backend. I have posted some thoughts & questions about it over on Gitter. You can also follow the progress of the project in my mssqlserver-serviceticketmanager repository. I welcome any feedback on how its development is coming along, especially suggestions on how to make it more robust and most useful to the masses. Also, I'm pretty sure that I'll need to rename it to somehow fall in line with it being a supplemental package of DotNetCasClient. Perhaps DotNetCasClient.MicrosoftSQLServer? At the expense of a long package name, dare we go with something like DotNetCasClient.State.<class name> to better indicate how these external packages are adding on to the DotNetCasClient package?

TheHokieCoder commented 6 years ago

Another FWIW, I have implemented a MS SQL service ticket manager in my own code that uses a VARBINARY column to store the serialized CasAuthenticationTicket object. It uses a binary formatter to serialize to an in-memory stream, and then the bytes of that stream are written to the VARBINARY column. Works like a charm so far!

I have done all of that development in a single project that I am working on as a single class in the .NET project, so I will (hopefully) soon migrate that development over to my aforementioned repository for the new project.

@phantomtypist Do you have any thoughts on the naming of the projects & NuGet packages for said service ticket managers? Also, how do you see us best managing these related projects? Should we start blank projects under the Apereo team and then allow the interested developers contribute to them? Or allow the initial non-Apereo team developers maintain a separately-owned project, and they wouldn't ever be officially supported under the Apereo umbrella?

phantomtypist commented 6 years ago

See my response on #84.... similar.

Make separate repo for each of the tertiary NuGet packages. This way they are isolated both by code and contributors. We could end up having different contributors on the different tertiary NuGet packages (e.g. someone might specialize in Redis and a different person on SQL Server implementations.)

This is somewhat similar to what Cake (the build scripting DSL we use) did with their plugins/addons: https://github.com/cake-contrib

They went a step farther and even created a separate GitHub Org (umbrella) to put these repos under, but I don't think we need to go that far.

@mmoayyed thoughts?

phantomtypist commented 6 years ago

We just have to agree on naming of the repos as per my comment in #84.

TheHokieCoder commented 6 years ago

For naming of the tertiary projects, I personally like something like DotNetCasClient.State.Redis to fall in line with the organization of the code within the project, and it gives a bit of scope for those who don't know what the project/package is beforehand. Something like DotNetCasClient.Redis might be assumed as a custom build of DotNetCasClient that only works with Redis, as opposed to a "plugin" for DotNetCasClient. But maybe I am over-analyzing it.

I, too, agree that having the repo name match the project/package name would have been ideal. Maybe that could be an option for the .NET Core version, to start off with a new repo named DotNetCasClient so that it is distinguished from the .NET <= 4.5 versions? It should be a drastically different codebase, right? Or better yet, DotNetCoreCasClient? 😆

Also, can we agree on a naming for the project/package for a MS SQL Server service ticket manager and have you create a skeleton project for that, too? That way I can start contributing my code to that and make it available for others to use. If you are OK with the above, I'd suggest DotNetCasClient.State.MSSQL, but I'll still name the class DotNetCasClient.State.MSSQLServiceTicketManager to be more explicit in code.

phantomtypist commented 6 years ago

@TheHokieCoder I had @mmoayyed set up the new repos. The SQL Server service ticket provider will live over in https://github.com/apereo/dotnet-cas-client-mssql

I'll scaffold out the project shell, solution, build scripts, AppVeyor CI and also the MyGet/NuGet feeds over the next week. After that we can retarget your code you already made over there.

Sound like a plan?

phantomtypist commented 5 years ago

@TheHokieCoder any progress on the MSSQL distributed cache provider?

TheHokieCoder commented 5 years ago

Yes, I had come up with a working example in a private repo. I apologize as I somehow missed your comment from last year about the separate repo.

In any case, a sticky point with implementing the provider is how to standardize the way the provider ties into the backend, which in this case is the MS SQL Server database. Some examples of how this could be done:

What are your thoughts on this? For my projects, it was easy to just hard-code the DB objects because I only need it once for my projects. But I am iffy about putting that out there to the world and mandating what the object names are.

phantomtypist commented 5 years ago

That's okay, I've been MIA too.

In no specific order:

Default appSettings applied via web.config.transform on NuGet package installation:

<add key="cas:MSSql:ConnectionString" value="" />
<add key="cas:MSSql:ServiceTicket:TableName" value="cas_st" />
<add key="cas:MSSql:ProxyTicket:TableName" value="cas_pgt" />

Other thoughts:

phantomtypist commented 5 years ago

^ w.r.t. the implementations take a look at the release/1.0 branch and look at what I did for the two classes (proxy and service tickets.)

An interesting problem I ran into that the original PR did not account for was that Memcached keys are limited to 250 characters whereas proxy/service ticket key length can be greater than that. I ended up going back to the actual CAS and Java client implementations to get some inspiration for a solution. I ended up taking a SHA512 of the pgt/st and using that as the key. Obviously you won't have that problem in SQL since you can make the column length larger than that ;)

TheHokieCoder commented 5 years ago

OK, sounds good! Thanks for the direction!

I would suggest we use cas:MSSql:ConnectionStringName as the appSetting key name to make it clearer that we are asking for the name of the key, not the connection string itself.

Everything else I am on board with.

As far as the Memcached key for the tickets, I found this in the CAS server docs:

6. Services MUST be able to accept service tickets of up to 32 characters in length. It is RECOMMENDED that services support service tickets of up to 256 characters in length.

I'd say the 250/256 character discrepancy would be negligible considering it is just a recommendation, and that the minimum requirement is 32 characters. I was going to suggest just putting a disclaimer on the provider that Memcached limits the key length, so the max supported ticket key length is 250, but then I realized that most consumers of the provider would most likely not have any control over the CAS server in question. So would it maybe be better to have the ability to hash the keys and make it a configurable option? That way, majority of the consumers would have a transparent mapping between client and server storage of keys, but the few that are using (insanely) long keys would be covered as well?

phantomtypist commented 5 years ago

Good catch, I wrote it too quickly. cas:MSSql:ConnectionStringName is what I meant to say.

As for the ticket character length it just so happened to be that my org has it's ticket length set to 256 :)

I figure making the key component non-configurable means a developer just doesn't have to worry about it. For example if the developer had to configure this manually in appSettings, let's say the sysadmins changed their mind and didn't tell the devops/developers about it then customers wouldn't be able to log in. Then someone has to RDP into a box and change the setting or do another build/release in a CI/CD pipeline to fix the problem. The few other clients I saw that did the Redis and Memcached implementations did not allow for this setting to be configured... they just hashed the ticket to a shorter length hash string. Some used FNV1, but I thought that was too short and could cause key collisions more often than a SHA512 hash.

phantomtypist commented 5 years ago

@TheHokieCoder FYI I think I have to quickly touch-up the Cake build script in the develop branch for you since Cake evolved a hair since I first put it in there. I'll do that now.

After that you should just be able to run the Cake script (from PowerShell) using the command .\build. It's set up to generate the NuGet package in an artifacts folder in the root of the repo.

I have @mmoayyed setting up integrations between GitHub and AppVeyor to automate the builds whenever you push to GitHub. Successful builds will push the NuGet artifacts to our MyGet CI feed for this package.

phantomtypist commented 5 years ago

FYI Memcached worked completed in new repo over at https://github.com/apereo/dotnet-cas-client-memcached/

DotNetCasClient.Memcached NuGet package now available

GitHub
apereo/dotnet-cas-client-memcached
Contribute to apereo/dotnet-cas-client-memcached development by creating an account on GitHub.
phantomtypist commented 5 years ago

FYI Redis work completed in new repo over at https://github.com/apereo/dotnet-cas-client-redis/

DotNetCasClient.Redis NuGet package now available

GitHub
apereo/dotnet-cas-client-redis
Contribute to apereo/dotnet-cas-client-redis development by creating an account on GitHub.
phantomtypist commented 4 years ago

@TheHokieCoder the only thing left on this issue to work on is the SQL Server provider. I think you said last time we spoke it was almost done? I'm fine leaving the issue under me, but if you want to take the football you can assign it to yourself.

TheHokieCoder commented 4 years ago

@phantomtypist I am fine with having this issue assigned to me. I'll get situated with where I left off and try to get this back on my radar so that we can have a (more) full selection of distributed cache providers. Thanks for bumping this for me.