Add SQL store option to token store

[imtayadeway]

This does a couple of few things:

1. Makes the token store honor the kind of session store that's been configured in the global settings
2. Adds support for a SQL session store, if that's been configured
3. Updates some existing tests to use both memory and sql to ensure good integration
4. Updates the purger to respect the ttl of the token, not the updated_on value

This is kind of a naive implementation that might have some issues. The most glaring is perhaps that we purge the Sessions based on a global ttl, meaning that this won't respect the ttl that is configured in the API/elsewhere, and that may not be easy to change. Opening this as a WIP to aid discussion

@miq-bot add-label api, core, enhancement
@miq-bot assign @gtanzillo

Fixes #14882
Fixes https://www.pivotaltracker.com/story/show/130490379

/cc @martinpovolny

[imtayadeway]

@miq-bot rm-label wip

[imtayadeway]

@gtanzillo @abellotti I've fleshed this out into something more workable. LMK what you think. Probably best reviewed ignoring whitespace.

There were quite a few opportunities to refactor along the way with this one but I resisted as it is already quite large. You may see some further duplication added (e.g. the knowledge of how we serialize/deserialize data in sessions) along the way....because this PR is already quite large I think it would be best if I tackled that in a follow up.

[chrisarcand]

Adds support for a SQL session store, if that's been configured

Unless I'm mistaken, isn't this what https://github.com/rails/activerecord-session_store does? Can't you utilize that instead?

[chrisarcand]

(we already use it in the project, I mean)

[imtayadeway]

@chrisarcand we do.....but that doesn't have an abstraction layer at the level we need. It just provides an adapter for action dispatch which interacts directly with the model. The token store code is coupled to this interface, which the other adapters provide, so I'm adding an adapter for that here.

[isimluk]

👍

[imtayadeway]

@gtanzillo @abellotti any feedback on this? It's currently red only because brakeman doesn't like the Marshal.load - that's somewhat unavoidable but in theory harmless because we what goes into that marshalled data is controlled by us. So we could whitelist that - I can go ahead and do that if the approach outlined above is agreeable.

[miq-bot]

Some comments on commits imtayadeway/manageiq@24422a3~...8715446

spec/requests/api/authentication_spec.rb

• ⚠️ - 175 - Detected expect_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.
• ⚠️ - 216 - Detected expect_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

[miq-bot]

Checked commits imtayadeway/manageiq@24422a3~...8715446 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
6 files checked, 4 offenses detected

app/models/session.rb

• ❗ - Line 31, Col 20 - Style/RescueModifier - Avoid using rescue in its modifier form.
• ❗ - Line 31, Col 28 - Security/MarshalLoad - Avoid using Marshal.load.

lib/token_store/sql_store.rb

• ❗ - Line 16, Col 22 - Security/MarshalLoad - Avoid using Marshal.load.

spec/lib/token_store/sql_store_spec.rb

• ❗ - Line 90, Col 13 - Security/MarshalLoad - Avoid using Marshal.load.

[imtayadeway]

@miq-bot add-label fine/yes

[simaishi]

@imtayadeway Is there a BZ for this? Can you please create if it doesn't exist?

[imtayadeway]

@simaishi here's the BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1459674 (hope I interpreted this correctly)

[simaishi]

Fine backport details:

$ git log -1
commit 9503a9a3ac59fa77f3a2cf72e5a9fcc13735bfeb
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Tue May 30 18:09:58 2017 -0400

    Merge pull request #14947 from imtayadeway/api/add-sql-store-to-token-store
    
    Add SQL store option to token store
    (cherry picked from commit ffd89722751ebc3fb0498a24a288b90411eea2ac)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1460348