Starcounter.Authorization should not create Starcounter.Session unless required
Opened this issue · 14 comments
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
"
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 BlendingEditor
s pages.
For now i see 2 differences, in the case of REST-API we have:
request.Method
isPUT
orDELETE
.request
hasAuthorization
header with valueBearer ...
.
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?
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.
@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.
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.
@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.
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
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.
@LarsTjernstrom could you debug Starcounter.Authorization
and identify the cause of the issue?
@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
@miyconst The root cause was Not Starcounter.Authorization, so we can delete those comments.
The root cause was that the customer wanted to be able to change the callback Url:s, so these could be edited from settings...but the app has to be restarted Or have code that does a new url registration. I have confirmed this with tests and by asking @alemoi.
E.g. if you have a variable in a Handle.POST(MyUrlVariable) a change of the variable will have no effect until the app is restarted Or you write code to specifically register it with the new url.
E.g. if you have a variable in a
Handle.POST(MyUrlVariable)
a change of the variable will have no effect until the app is restarted Or you write code to specifically register it with the new URL.
@LarsTjernstrom you can always register a dynamic URL, for example:
Handle.GET("/app/callback/{?}", (string uri) =>
{
if (uri != this.GetCustomerCallbackUri())
{
return 404;
}
// Process callback.
return 200;
});
Hi @miyconst, @mtornwall, @habib535 could you please share your vision on the things that are left here to do? I'm not sure on 100% that i have clear understanding.
By the information from Konstantin:
Basically, what we want, is to exclude auth request filter for some of the handlers.
Which is currently impossible.
So this is on hold for now.