API tokens persistance

[Cazzar]

Currently, when someone logs into Shoko with the API tokens, they get a newly generated token but, when this is the case, the token might not propelrly be disposed of, and when next needing it, just grabbing a new token.

Solution

To fix this up, I propose adding a "timeout" function to the tokens, effectively defaulting them to only last for a certain amount of time, but it being requestable as part of the login, ex

{
    "user": "Default",
    "pass": "",
    "device": "Shoko Example Login",
    "lifetime": 43200
}

in this example, I am using 43200 seconds, or 1 day.

[Cazzar]

I also do suggest having the options, such as <= 0 being "don't expire"

[da3dsoul]

I think it should have a default time, and the parameter is optional. It also resets the time when you connect with the token, so you don't need to reauth all the time.

[Cazzar]

Default and optional, sure that was my intention, as for the resetting timeout, logical.

[bigretromike]

just implement full OAuth2.0 as this is half done if you want to do anything with tokens ;p

[bigretromike]

ALSO THIS IS THE NUMBER OF THE BEAST

[hidden4003]

Also 1 day is 86400 seconds.

[hidden4003]

OAuth has permanent tokens that behave the way @Cazzar described, but also there are session tokens that always have time limit.

[bigretromike]

also each user + device combo get new token, not user. also they are not dispose, because they are used instead of login/password. Im not sure what usage you will get with expiring tokens. As I would understand to use those with webservice I dont see much use of those here. But if you want to implement it I wont protest. Just make it require so there is no legacy way to login/use api, because that would make no sense ;-)

[Cazzar]

This is the issue primarily

[bigretromike]

oh so you using same user + device... then its broken,. I will fix this if you want 👍 because I made it but it looks like it been broken - strange because when I put it there it was working fine. 👍

[bigretromike]

The intended way was it would look up if there is record for user + device and return it or create new if there is none

[Cazzar]

Each of those CLI tokens are bring created anew to the exact same auth string. And it's my Shoko graphing script

[bigretromike]

imo your script should use token not login/password. but notheless it shouldnt create new once.. will loook into this

[Cazzar]

Though whilst yes that would improve things in the short term adding the initial goal would be good as well.

On Sun., 17 Sep. 2017, 2:29 am BigRetroMike notifications@github.com wrote:

The intended way was it would look up if there is record for user + device and return it or create new if there is none

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ShokoAnime/ShokoServer/issues/666#issuecomment-329979013, or mute the thread https://github.com/notifications/unsubscribe-auth/AA8rppfb_2lgQQ7VIR-W8bdeZwoVJ2r1ks5si_dRgaJpZM4PK74i .

[Cazzar]

Also as per the comment about should use it, I make my scripts on the most case only need the API key generated once on startup, if the script ever crashes or exits it will re log in, it was designed in a way to be easily portable between instances.

[bigretromike]

I tested this multiply times. I don't get different key for same user/password/device combo. Looks like something with you script. I'm closing this as this is not a issue. You could share your script or part of it that is the smallest code that trigger the issue (if any). I even restart Server few times. Still getting same key

[da3dsoul]

More info. This seems to happen most when there's a lot of requests. @Cazzar had said this on discord if I recall correctly

[bigretromike]

if thats the case maybe its async issue, why not revert that to the way it was before async, now it looks like async (x, ct) => await Task.Factory.StartNew(() so I assume each request create Task etc. why not do it right away ? Im spaming server, but still no issue. How many is a lot ?

[da3dsoul]

A lot is 10 per second

[da3dsoul]

And it needs to be async, it just needs to handle it better behind the scenes

[bigretromike]

how better? it just select id from user where login="username" and password="pass" and if there is ID it select authkey from authkeys where id = "id" and device = "device" How better you can handle that ? maybe the Factory are dropping connection with db and it end up returning Null/None rows from db so server add new row. We could make a "uniq" device column but that would make you use new apikey for each script/device etc.

[da3dsoul]

Because the SQL is not async

[bigretromike]

Because they SQL is not async

Any MySQL client that supports the X Protocol can provide asynchronous execution, either using callbacks, Promises, or by explicitly waiting on a specific result at the moment in time when it is actually needed. https://dev.mysql.com/doc/x-devapi-userguide/en/synchronous-vs-asynchronous-execution.html

Even if we dont use async, then it time out and return null if that the case.

[da3dsoul]

We don't call the DB async, regardless of the DB implementation

[bigretromike]

So the problem is to recognize none - time out vs none - normal result

[Cazzar]

Actually there's 2 parts of this issue

The first part, token duplication is fixed, the next part would be a token lifetime, or "valid to" One possibility is moving to things like a JWT token

On Sun., 4 Feb. 2018, 7:21 am BigRetroMike, notifications@github.com wrote:

So the problem is to recognize none - time out vs none - normal result

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ShokoAnime/ShokoServer/issues/666#issuecomment-362851143, or mute the thread https://github.com/notifications/unsubscribe-auth/AA8rphoxrLkwRa4CncKHO1yH5omtN0Z-ks5tRL-4gaJpZM4PK74i .