Bug 103825 - [WOPI] access_token_ttl handling
Summary: [WOPI] access_token_ttl handling
Status: NEW
Alias: None
Product: LibreOffice Online
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.4.0
Keywords: needsDevAdvice
Depends on:
Blocks:
 
Reported: 2016-11-10 08:38 UTC by Aleksander Machniak
Modified: 2017-04-24 13:57 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Aleksander Machniak 2016-11-10 08:38:04 UTC
I found some references to access_token_ttl in the code, but it looks like it's not really implemented.

Note: access_token_ttl value is not really a time interval, it's a precise data/time of when the session is going to expire.

WOPI spec. talks about prompting users to refresh their sessions, but I think we can do this automatically. The editor could just send CheckFileInfo request to the WOPI service so it can refresh the session.

There's no information on how the editor can gen a new session_token_ttl value after refresh. I suppose the editor could calculate the interval from initial ttl value and refresh the session periodically using this interval.

http://wopi.readthedocs.io/projects/wopirest/en/latest/concepts.html?highlight=ttl#term-access-token-ttl
Comment 1 Pranav Kant 2016-11-24 16:22:35 UTC
(In reply to Aleksander Machniak from comment #0)
> WOPI spec. talks about prompting users to refresh their sessions, but I
> think we can do this automatically. The editor could just send CheckFileInfo
> request to the WOPI service so it can refresh the session.

How do you mean it, please ? Access token is provided by the WOPI host when loading the iframe (which is then passed to the server). Also, its not the editor that send the CheckFileInfo request to WOPI service, but the server (loolwsd) which interacts with wopi service.
Comment 2 Aleksander Machniak 2016-11-24 19:05:24 UTC
(In reply to Pranav Kant from comment #1)
> How do you mean it, please ? Access token is provided by the WOPI host when
> loading the iframe (which is then passed to the server).

Yes.

> Also, its not the
> editor that send the CheckFileInfo request to WOPI service, but the server
> (loolwsd) which interacts with wopi service.

Sorry, by "the editor" I meant loolwsd server.
Comment 3 Pranav Kant 2016-11-28 11:04:41 UTC
I am still not clear about how we would be able to do this automatically without prompting the user. 

On the server side, when access token is about to expire, sure I can request CheckFileInfo from the WOPI service but what good that would do ? My plan was to intimate the user when we are near access_token_ttl who would then provide loolwsd with a new access_token that loolwsd can reliably use for WOPI operations (till ttl expires again). But how would I get this new access token from WOPI service by making a CheckFileInfo request (loolwsd -> WOPI service) ?

If you can explain your idea a bit further, that would be great.
Comment 4 Aleksander Machniak 2016-11-28 11:28:46 UTC
(In reply to Pranav Kant from comment #3)
> On the server side, when access token is about to expire, sure I can request
> CheckFileInfo from the WOPI service but what good that would do ? My plan

The CheckFileInfo would be a ping to the service/session, so it knows it's still in use and should not expire the session token.

> was to intimate the user when we are near access_token_ttl who would then
> provide loolwsd with a new access_token that loolwsd can reliably use for
> WOPI operations (till ttl expires again). But how would I get this new
> access token from WOPI service by making a CheckFileInfo request (loolwsd ->
> WOPI service) ?

I don't really see a need to make user aware of this internal process. Also, I don't see a way to pass the new token from WOPI to loolwsd. That's why I proposed to ping the WOPI service so it does not expiry the initial token.
Comment 5 Pranav Kant 2016-11-28 11:43:48 UTC
(In reply to Aleksander Machniak from comment #4)
> (In reply to Pranav Kant from comment #3)
> > On the server side, when access token is about to expire, sure I can request
> > CheckFileInfo from the WOPI service but what good that would do ? My plan
> 
> The CheckFileInfo would be a ping to the service/session, so it knows it's
> still in use and should not expire the session token.
> 

I am bit skeptical w.r.t security here. What if the access token is leaked somehow. I am inclined towards not extending the existing token so that even if token is compromised, the attacker is not able to abuse the WOPI service for indefinite period.

> > was to intimate the user when we are near access_token_ttl who would then
> > provide loolwsd with a new access_token that loolwsd can reliably use for
> > WOPI operations (till ttl expires again). But how would I get this new
> > access token from WOPI service by making a CheckFileInfo request (loolwsd ->
> > WOPI service) ?
> 
> I don't really see a need to make user aware of this internal process. 

My guess is that this is also enforced by WOPI specs. due to the security concern mentioned above.

> Also, I don't see a way to pass the new token from WOPI to loolwsd. 

If we refresh the page, it would go through the same cycle again. That is one solution.

Other might be that I create another post message API where WOPI host inserts its new token and loleaflet then passes the access token to loolwsd internally through websockets.
Comment 6 Aleksander Machniak 2016-11-28 12:10:28 UTC
(In reply to Pranav Kant from comment #5)
> If we refresh the page, it would go through the same cycle again. That is
> one solution.

> Other might be that I create another post message API where WOPI host
> inserts its new token and loleaflet then passes the access token to loolwsd
> internally through websockets.

If you provide postMessage I can do the token refresh from the host page. Actually I can do this currently, but it depends on what loolwsd does with session_token_ttl (i.e. if it cares about it at all).

So, that could work too. I could live with that.
Comment 7 Commit Notification 2016-12-02 12:39:39 UTC
Pranav Kant committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/online/commit/?id=dde653f9208342f42ea207cb840a27bb089d7347

tdf#103825: Prompt the user when session is about to expire
Comment 8 Aleksander Machniak 2016-12-02 12:57:44 UTC
I just saw "access_token_ttl must be a unix timestamp of when token will expire" in the above commit. This is not really true as for WOPI it is a number of milliseconds (not seconds) since 1970-01-01.

Also, the hardcoded 15 minutes time is well, not what I'd expect. I can imagine implementations with small session lifetime. E.g. PHP's default is 24 minutes. So, maybe it should be something like min("15 minutes", (ttl-now)/2).

Finally, as I really want to prevent from displaying this warning to the user, I hope you'll implement a way to reset the token and timer with postMessage API.
Comment 9 Pranav Kant 2016-12-02 17:16:16 UTC
(In reply to Aleksander Machniak from comment #8)
> I just saw "access_token_ttl must be a unix timestamp of when token will
> expire" in the above commit. This is not really true as for WOPI it is a
> number of milliseconds (not seconds) since 1970-01-01.

Yes, it is the number of milliseconds actually, ignore the commit message then - it is incorrect.

> 
> Also, the hardcoded 15 minutes time is well, not what I'd expect. I can
> imagine implementations with small session lifetime. 

> E.g. PHP's default is
> 24 minutes. So, maybe it should be something like min("15 minutes",
> (ttl-now)/2).

Oh, didn't know that.

> 
> Finally, as I really want to prevent from displaying this warning to the
> user, I hope you'll implement a way to reset the token and timer with
> postMessage API.

Yes, pushing a safe solution just before the release with minimum changes in the code, surely will pick it up after the release.
Comment 10 Xisco Faulí 2017-04-13 08:42:58 UTC
Putting back to NEW as there's no assignee to this bug