Starcounter.Authorization should not create Starcounter.Session unless required

[miyconst]

It turns out that Starcounter.Authorization will create new Starcounter.Session for every request, even if it's a simple REST API request.

This is very inefficient, because every REST API request would create a new Starcounter.Session which won't be used any more, but will consumer resources and eventually die with a timeout.

This session spawn leads to the following exception under high load of REST API.

"System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values.
Parameter name: No free session indexes left.
   at Starcounter.Internal.SchedulerSessions.CreateNewSession(ScSessionStruct& ss, Session publicSession, UInt64 timeoutMinutes) in C:\    c_work\sc-19961\Level1\src\Starcounter.Sessions2x\SchedulerSessions.cs:line 112
   at Starcounter.Sessions2x.SessionFactory.Create(Session publicSession, ScSessionStruct& sss) in C:\    c_work\sc-19961\Level1\src\Starcounter.Sessions2x\SessionFactory.cs:line 20
   at Starcounter.Internal.XSONSessionDependencies.SessionFactoryWithSchedulers.Create(Session publicSession) in C:\    c_work\sc-19961\Level1\src\Starcounter\Apps\XSONDependencies.cs:line 93
   at Starcounter.Session..ctor(Flags flags, Nullable`1 includeNamespaces) in c:\github\Starcounter.XSON\src\Starcounter.XSON\Sessions\Session.cs:line 184
   at Starcounter.Session.Ensure() in c:\github\Starcounter.XSON\src\Starcounter.XSON\Sessions\Session.cs:line 373
   at Starcounter.Authorization.Authentication.TicketCreationStartupFilter.<>c__DisplayClass2_0.<Configure>b__1(Request request)
   at Starcounter.Handle.RunRequestFilters(Request req) in C:\    c_work\sc-19961\Level1\src\Starcounter.Internal\Handle.cs:line 555
   at Starcounter.Internal.AppsBootstrapper.ProcessExternalRequest(Request req) in C:\    c_work\sc-19961\Level1\src\Starcounter.Apps.JsonPatch\AppsBootstrapper.cs:line 345
ParamName=No free session indexes left.
HResult=-2146233086
"

[kegor]

I checked request object on this line https://github.com/Starcounter/Starcounter.Authorization/blob/3.x/Authorization/Authentication/TicketCreationStartupFilter.cs#L21 during REST-API requests mentioned on this page https://github.com/Starcounter/Blending/blob/master/docs/administration.md (as an well-known example for me) and during regular get request to one of the BlendingEditors pages.

For now i see 2 differences, in the case of REST-API we have:

1. request.Method is PUT or DELETE.
2. request has Authorization header with value Bearer ....

So for now as the solution i could suggest to use one of these options to filter such requests on the same line (https://github.com/Starcounter/Starcounter.Authorization/blob/3.x/Authorization/Authentication/TicketCreationStartupFilter.cs#L21) as we do for the request.IsStaticFileRequest, but i'm not sure that this is the best solution. @miyconst could you please share your opinion?

[miyconst]

We can't filter API requests by the HTTP method. There are APIs which are GET as well.

I think, that Starcounter.Session and all sign in info (even if user is not signed in) shall only be created if requested.

Like a lazy loading, don't create it unless someone is trying to fetch it.

REST API may also use Starcounter.Session and rely on Starcounter.Authorization, but there should be a way to opt-out and don't waste resources creating Starcounter.Session when it's not needed.

[LarsTjernstrom]

Hi @kegor.
Yes as @miyconst writes, there is no need to create a Starcounter.Session in Starcounter.Authorization when it’s rest call and not a viwmodel call.

Then the question is, what is the most simple way in Starcounter to know if the call i
just a rest call
Or a ViewModel call?

[LarsTjernstrom]

@miyconst wrote. Rest API handler is not necessarily returns JSON, it may return nothing, just a string, xml, or even a view-model, which is going to be serialized into JSON.

[habib535]

Since its the Starcounter.Startup library that registers all the handlers from viewmodel, one way is to expose an extra flag that will set SkipRequestFilters = true there https://docs.starcounter.io/topic-guides/network/middleware#skip-request-filters

This can also have side effects though.

[LarsTjernstrom]

@habib535 in the case of Rest calls many times there will be no ViewModel...so we can't add a property that says skip me..when the viewmodel itself is not needed when doing rest calls.

[LarsTjernstrom]

Hi everyone, @kegor , @miyconst . In Cronofy I had Rest callbacks from Cronofy working before, but now they are blocked and never reach my code, Cronofy gets a 404 response.
I'm thinking of opt in vs opt out...We would like the authorization to protect our viewmodels when we have added an Authorize Attribute, if it's missing then access should be allowed = then it will work for Rest calls

[kegor]

Hi guys, on this line https://github.com/Starcounter/Starcounter.Authorization/blob/3.x/Authorization/Middleware/SecurityMiddleware.cs#L33 method CheckClass returns true (https://github.com/Starcounter/Starcounter.Authorization/blob/3.x/Authorization/PageSecurity/CheckersCreator.cs#L164) if there is no Authorize attribute found on the Type pageType, and therefore this middleware just returns next() in this case, i could re-test this part if you assume that this might work wrong.

[miyconst]

@LarsTjernstrom could you debug Starcounter.Authorization and identify the cause of the issue?

[habib535]

@miyconst @LarsTjernstrom
If the goal of this issue to discuss how to skip authorization for particular requests then how #91 (comment) is not solving this issue? Its a middleware in the Authorization library that creates the session and go through the authorization code on every request. https://github.com/Starcounter/Starcounter.Authorization/blob/3.x/Authorization/Authentication/TicketCreationStartupFilter.cs#L19

If the request is not from a viewmodel (Startup library) but our own Handler.GET|POST etc then we can add the SkipRequestFilters = true ourselves.

@kegor
Please have a look at the habib branch which has a fix for the same 59d0f7f