Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
phantomtypist opened this issue Jul 19, 2017 · 23 comments
Open
Assignees

Comments

@phantomtypist
Copy link
Contributor

phantomtypist commented Jul 19, 2017

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

  • Redis done
  • Memcached done
  • SQL Server
@phantomtypist
Copy link
Contributor Author

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

@sunnymoon
Copy link

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
Copy link
Contributor Author

phantomtypist commented Aug 11, 2017

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

@phantomtypist
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@TheHokieCoder
Copy link
Contributor

TheHokieCoder commented Apr 19, 2018

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

@TheHokieCoder any progress on the MSSQL distributed cache provider?

@TheHokieCoder
Copy link
Contributor

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:

  • the provider requires a provided DB connection (to any database) and then specifies what are the table names, column names, etc
  • the provider requires a bunch of app settings to define the database objects that are needed and just calls those dynamically
  • the provider gives interfaces for providing/storing the required data, and it is up to the developer to implement the methods to connect specifically to their database, leaving the door wide open to naming the DB objects however they want.

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
Copy link
Contributor Author

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

In no specific order:

  • Table names: I looked at some other implementations from other languages and it looks like the two tables (proxy granting tickets and service tickets) would be named cas_pgt and cas_st if we wanted to just piggy back on what other people have leaned towards. I think those could be the suggested values, but I get why people might want to name them different in their own projects (more below.)
  • Column names: I'd say lets hard code these because we are allowing people to at least change the table name. I can't remember the table names from last night, but I can go back and see what they were using if, but I'm sure yours will be okay though.
  • Connection string: Just reference via an appSetting variable the name of the actual connectionString entry to use (more below.)
  • AppSettings configuration: I'm going to suggest we "namespace" some appSettings for us instead of putting them inside the casClientConfig block. 1) It's just easier from an out-of-the-box perspective 2) easier to do web.config.transform transformations when installing the NuGet package and 3) easier for deployment systems (e.g. Octopus) to manipulate appSettings than anything else.

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:

  • For the Redis and Memcached versions I am only going to supply .NET 4.5+ assembly compatibility. For this one, I guess you could actually get away with doing .NET 2, 3.5, 4, and 4.5 just like the main project/repo since you'll just be referencing System.Data.SqlClient. I guess it would be a nice thing to give people with those old projects at least one type of distributed caching support.
  • I'd say implement the actual SqlConnection, SqlCommand, SqlParameter, etc. calls inside the two class files I scaffolded for you. Fully implement them so the consumers don't have to implement anything.

@phantomtypist
Copy link
Contributor Author

^ 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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

phantomtypist commented Apr 15, 2019

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

DotNetCasClient.Memcached NuGet package now available

GitHub
Contribute to apereo/dotnet-cas-client-memcached development by creating an account on GitHub.

@phantomtypist
Copy link
Contributor Author

phantomtypist commented Apr 16, 2019

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

DotNetCasClient.Redis NuGet package now available

GitHub
Contribute to apereo/dotnet-cas-client-redis development by creating an account on GitHub.

@phantomtypist
Copy link
Contributor Author

@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
Copy link
Contributor

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants