search

ERP-Ukraine / odoo-rpc-dart

Odoo RPC Library for Dart
MIT License
44 stars 35 forks source link

Cookie for Auth #11

Closed anexa-digital closed 3 years ago

anexa-digital commented 3 years ago

Hi. Great job.

I have found 2 issues:

  1. On odoo_client.dart / _updateSessionIdFromCookies , data from response.headers['set-cookie'] comes with an error from Odoo response after authentication (I have tested with Community Ed. Versions 12 and 14).

Content of set-cookie: __cfduid=d7aa416b09272df9c8ooooooo84f5d031615155878; expires=Tue, 06-Apr-21 22:24:38 GMT; path=/; domain=.mhfly.com; HttpOnly; SameSite=Lax,session_id=16ec9824767d8ca27ooooooo2fa61803fb35857b; Expires=Sat, 05-Jun-2021 22:24:38 GMT; Max-Age=7776000; HttpOnly; Path=/

Note that separator between SameSite=Lax AND session_id =... is a comma, not a semicolon. This Odoo bug cause bad parsing in _updateSessionIdFromCookies

  1. For a good callRPC you must send __cfduid + session_id in Cookie, something like: __cfduid=d0c6c49b60848cc2d509ooooooo1261615152080; session_id=5af3e0bb806a96b7oooooooo5398bc4be1a25

Even sending whole original set-cookie obtained from authentication also works (just changing comma separator by semicolon).

Best regards.

lem8r commented 3 years ago

Hi!

Thanks for reporting. That's interesting topic.

According to RFC 6265 the only valid separator for set-cookie is semicolon.

SameSite cookie is not added by Odoo. See https://github.com/odoo/odoo/issues/51065 Maybe your frontend is adding it and it is possible to change separator it uses?

CF is dropping __cfduid so not worth bothering https://blog.cloudflare.com/deprecating-cfduid-cookie/

lem8r commented 3 years ago

It might be dart's issue. Headers with same name are joined with comma https://github.com/dart-lang/http/blob/master/lib/src/io_client.dart#L43

lem8r commented 3 years ago

@anexa-digital please try version 0.4.3. It should handle you case now.

Except second point. Setting all cookies requires proper handling of cookie rules as SameSite or Expires. It may be required for cases like Sticky Session. It should be done as separate feature not a fix commit.

anexa-digital commented 3 years ago

Works great! Thanks a lot.