Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🥔✨ Marketplace: Encrypt and Store Square Access Token #1731

Merged
merged 9 commits into from
Aug 8, 2023

Conversation

rosschapman
Copy link
Contributor

@rosschapman rosschapman commented Aug 4, 2023

I borrowed this pattern for storing encrypted data from Utility, and I think it works well for Furniture-level private keys as well. In order not to conflict with the more general settings attribute, I opted for secrets.

@rosschapman rosschapman requested a review from a team August 4, 2023 01:42
@rosschapman
Copy link
Contributor Author

rosschapman commented Aug 4, 2023

@zspencer @anaulin I can't figure out why the rspec test if failing with a MissingAttribute error for the new field. Getting the same thing locally. I see that the new migration has been applied and the new column exists in the test database. May need some help. 😩

@zspencer zspencer changed the title Adds ability to store Square Access Token on a Marketplace 🥔✨ Marketplacae: Encrypt and Store Square Access Token Aug 6, 2023
@zspencer zspencer changed the title 🥔✨ Marketplacae: Encrypt and Store Square Access Token 🥔✨ Marketplace: Encrypt and Store Square Access Token Aug 6, 2023
Copy link
Member

@zspencer zspencer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I did some digging and figured out why it was not working.

The TL/DR is that RankedModel uses Furniture.select(:id, :room_id, :slot) under the covers at some point; which retrieves only those three attributes.

By adding an if has_attribute?("secrets_ciphertext") we skip setting the default in cass where the secrets are not loaded.

@rosschapman
Copy link
Contributor Author

rosschapman commented Aug 8, 2023

Thanks for the assist @zspencer. Whew. I'm now more well-read on Ranked model, too. It's gotta be this line in the gem: https://github.com/brendon/ranked-model/blob/f9145d42041ed7186a893315350f01f87d39c76b/lib/ranked-model/ranker.rb#L276. I haven't traced beyond that.

Comment on lines +33 to +38
# The `RankedModel` gem uses
# [`ActiveRecord::QueryMethods.select`](https://api.rubyonrails.org/classes/ActiveRecord/QueryMethods.html#method-i-select)
# to do some stuff (Validation?) under the covers while keeping memory usage light... Which is good!
# But it also means that we don't have the ciphertext!
# So we can't set the secrets in that case (nor would we want to)
self.secrets ||= {} if has_attribute?("secrets_ciphertext")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zspencer Makes me think this will not be the last time we try to access data on models returned ranked queries with surprising results 🤣

@rosschapman rosschapman merged commit cbd9044 into main Aug 8, 2023
@rosschapman rosschapman deleted the adds-encrypted-configuration-to-furniture branch August 8, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants