New project maintainer/owner required #379
Comments
|
I apologize for not being a great maintainer over the past couple years. Life has caught up with me. As @sbfaulkner said, it may be time for another maintainer to step up. I will do my best to keep things moving, but I really don't have the time anymore. If you feel like you can handle this project, please reach out to me. |
|
We could just support another library? I've used https://github.com/ankane/lockbox looks promising. |
|
Note: This gem attr_encrypted does work with Rails 6.1 (!). My apologies, I misunderstood an earlier error. @dudo : My existing application depends on attr_encrypted. It'd be a pain to rewrite it for a different API. I'd prefer to keep using the existing attr_encrypted API (in part because most code "just works" correctly) instead of trying to rewrite existing code for a new API. |
This commit *begins* an update from Rails 6.0.X -> 6.1.X. This requires a large number of simultaneous library updates (some took a while to identify). This does *not* complete the update. The biggest problem is the many error reports of this form: > Error: > ProjectsControllerTest#test_should_fail_to_create_project_with_duplicate_repo: > ActiveRecord::StatementInvalid: PG::UndefinedColumn: > ERROR: column users.email does not exist It's true that there is no users.email column in the database, but that's because it's a virtual column that is *supposed* to be managed by the `attr_encrypted` gem. Since it's not being handled, it *appears* that this gem does not work with ActiveRecord 6.1. I went to check out its status, and they are looking for new maintainers: attr-encrypted/attr_encrypted#379 All options are not the desired ones here. In addition, there are at least two kinds of deprecation warnings which will need to be addressed (probably many times): * DEPRECATION WARNING: action_view.raise_on_missing_translations is deprecated and will be removed in Rails 6.2. Set i18n.raise_on_missing_translations instead. Note that this new setting also affects how missing translations are handled in controllers. (called from call at /home/dwheeler/best-practices-badge/config/initializers/canonical_trailing_slash.rb:30) * DEPRECATION WARNING: Rendering actions with '.' in the name is deprecated: static_pages/error_404.html.erb (called from error_404 at /home/dwheeler/best-practices-badge/app/controllers/static_pages_controller.rb:46) Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
|
Note: This gem attr_encrypted does work with Rails 6.1 (!). My apologies, I misunderstood an earlier error. It definitely should keep going. Also: I think it's reasonable to have this gem not have a lot of functionality. "It does this one thing well" is perfectly reasonable. |
|
@david-a-wheeler I don't think anyone here is saying that this project should die, the maintainer is asking for someone to pass the torch onto. I have been on both sides, it can be bittersweet. I have maintained thousands of open source projects and still maintain a fair number when I can spare the time. It is a lot of work and it's not fair to assume that because someone started or maintained a project at one point that they will be in a position or want to do that forever. I would like to thank all the amazing folks who put in their time into building something that so many have relied on over the last decade. This includes maintainers, code contributors, bug reporters, feature requesters, etc. |
|
I also want to greatly thank all the amazing people who've put work into this! |
|
Hi @sbfaulkner, We have actually been using this gem in-house since we began in 2015, and are -- obviously --quite fond of it. (Incidentally I believe one of our founders is a former colleague of yours'.) In any case, given the nature of our business, we are very passionate about keeping our (and our users') data secure. We would love to, in this small way, help the greater community do the same. As we would need to maintain this gem anyway for our own use, this seems like almost a perfect fit. I'd love see if we can make this happen and would be more than happy to discuss a possible transition. I'm not sure what medium would be preferable for you, but please let me know and I can make it work. * As a bit of background, our firm helps employees achieve financial security by being able to access their pay right after they've earned it, instead of waiting for a payday. We don't do loans and we don't charge any sort of interest. We work directly with employers to integrate into their payroll systems and our service is offered as a benefit to employees. If you're curious, more info about us/what we do can be found on our website. |
|
thanks for your offer @mvastola I think the primary qualification we have for this is interest, so this is great news! I'm pretty much open to ideas regarding the transition, and figure we can probably manage it here in this issue. @saghaulor as the most recent steward, I'd appreciate your input on how we should do this. As a start, in no particular order, we'd need to deal with...
if anyone comes up with other things we'll need to address, please comment here ❤️ |
|
@sbfaulkner Awesome! I can't think of anything major. The only additional changes I can think of that might come up are:
We haven't gotten into too much detail on the logistics of this internally though (other than being enthusiastic about extending this offer), so these are just guesses. Given your interest, I'll investigate this further, and we obviously won't be making any changes until then. |
|
@mvastola that's great news! Don't you think it would be better to keep the project under the current org, so that no particular company owns it? Then if you end up in our situation where you just can't find the time to dedicate to the project, you don't have to worry about an org transfer, you just request that you be removed as maintainers? I worry that a particular company owning it will cause it end up like Paperclip, where everyone thinks it's dead and then another org must fork it to continue the work. I don't have any particular affiliation with Travis CI, but it seems to have done a good enough job. Is there a reason to change things? My only input would be that I tried to make this gem be as simple and most secure to use as possible. That being said, having a bunch of people that can push to rubygems reduces the potential security of the gem. I would say that we should be careful about how generous we are giving access to rubygems. The same could be said for Github, but at least then you can see the source code before you pull it. It's possible that someone can push to rubygems and not github, removing transparency. Of course once you pull from rubygems you can inspect the source, but I don't know that a lot of people do that. Related, I tried to sign the releases, but I don't think anyone checks that stuff. When I asked around there were only a handful of projects on rubygems that actually signed their work. The signing process is weird because you have to sign, then push the code, then push the signatures after, or something like that. I can't remember the order of operations, but it was a little tricky to get right. I had plans to do a major code cleanup in the next major release, dropping some functionality, and cleaning up the tests. People have tried to drop support for legacy releases of Ruby and Rails, but I kept that code around because it didn't muddy things up that much, and it's nice to have a library that works across multiple major releases of Ruby and Rails, so that upgrading is rather painless. Should you become the new maintainers, it's obviously up to you how you want to do things, I'm just giving my approach to things as @sbfaulkner requested. |
|
@saghaulor, to reply to your points: OrganizationIn talking to my team I apparently spoke to soon re: those two predictions. We are not planning on making any of those changes. I had just assumed we'd want to. This is our first foray into the open source world, so I didn't have any precedent to go by. I was just using what we do for other projects and what other gems do as a frame of reference. For what it's worth, even if we had done this, we certainly wouldn't consider it to be owned by us (as that's kind of antithetical to the idea of OSS), and I would be very shocked if there were ever any consideration of us holding this hostage if we could no longer support it, whether or not we moved organizations on it. Part of my thinking on this was that we would make it match the way we work internally, and internally we work with branches instead of forks. But, as far as I know, our approach is going to be to not change anything that isn't broken. Along the same lines, we use a CI service other than Travis, which is the only reason I thought we might change from them. SecurityOur approval pipeline gives maybe half a dozen people (max) the ability to deploy releases as well as to merge to our master branch. I'm not sure how many people will actually end up working on this (though obviously we'll make sure all PRs/issues get fielded in a timely manner) so I doubt we'd have a "too many cooks" issue there. I'll see if we can get signing figured out. (It's a nice-to-have, even if it's not widely used yet.) It's also possible our devops people would be willing to construct a deployment pipeline for the gem that might make signing and releasing more consistent/automated, but I don't want to make any promises I can't keep. Lastly, to be clear, regarding for any future work you are hoping to do, we would definitely love you to still be involved to whatever extent you'd like. (This goes for the current maintainers as well.) We're certainly not looking to push anyone out. |
|
Oh, and as for Ruby versions, I imagine we're going to take the same approach if it doesn't require extra work. I know I've had issues compiling older versions of ruby on newer OSes (I think the |
|
my two cents here about ruby versions, in most cases we should not care about 2.4 and below. about this project's future, I hope its remains on the current umbrella moving git repos and updating URLs to new owner requires a good motivation and I don't see a lot of benefit, especially this one is already under a team name, not an individual account. |
|
Regarding version support I recommend rather than discuss specific versions we come up with some generic policy we can adhere to going forward. Maybe something along the lines of |
|
my recommendation (to be taken with a grain of salt given that I'm backing away from the project) is to follow actual ruby EOL dates per link provided by @majormoses and support any given version only up until the EOL date... |
|
invited you to the org @mvastola |
|
@sbfaulkner, thanks! I'll be having a meeting later this week to determine our internal logistics on this, so I don't anticipate anything significant happening for a few days. Re: Ruby versions, it looks like there isn't a clear consensus (except that we should obviously continue maintaining support for versions that aren't EOL). I imagine we'll end support for EOLed versions at the point where it becomes cumbersome. At some point we'll need access to rubygems.org too but doesn't have to be right this second. Do you happen to know which (if any) of the current admins will want to remain involved with commits and such? I know @saghaulor mentioned having some plans to refactor, but I'm not sure if I should take it that those plans have since been scrapped. Thanks for your faith in me/us! We plan to do well by this project, and we are more than open to any further input or wisdom you guys have as things progress. |
|
Oh, do you guys have a slack or anything else you might use? (Perhaps a shared email for anyone reporting vulnerabilities?) |
|
no slack or even shared email... we just leaned on github issues if you comment here for rubygems access, we can navigate that then not sure any of us other than @saghaulor has been engaged in ages (outside of this issue) |
|
I mean I'm happy to set things up now if you time. It's obviously something we'll need eventually. |
|
@mvastola sure... what's your rubygems handle/account or the email I can assign as an owner on rubygems? |
|
That is me, but please put it under DailyPay (which I just created). |
|
added DailyPay to attr_encryptor and encryptor gems |
|
Thanks so much. Looks like it's working. |
that wasn't me ... I think it was saghaulor |
|
Just a heads up that an encryption API for ActiveRecord was recently merged and slated for release in Rails 7. rails/rails#41659 It appears that both |
|
Interesting. I also index my encrypted values, including ensuring that records that meet certain conditions are unique. I think it would work with Rails 7, but not certain. Hmmmm! |
|
@david-a-wheeler There are options in the new AR::Encryption stuff for deterministic encryption that would be indexable and there's a uniqueness validation for such. It's a pretty flexible infrastructure from what I can see. |
|
Yeah, we saw that. We're definitely going to ensure compatibility (or perhaps simplify a transition to the native feature) in RoR 7. |
|
Hi @mvastola ! Thanks for taking on maintaining this gem! I was just wondering what your plans are for the open issue/PR queue, particularly around Rails 5.2/6.x support which is why many of us are running off forks in production. We also have some in-progress support for key rotation that we'd like to contribute but it doesn't really make sense until the project is shored up with proper Ruby/Rails version support and such first. |
As noted in attr-encrypted#379 (comment), in Rails 7 uses a class method named `encrypted_attributes`, which clashes with the `encrypted_attributes` method used in `attr_encrypted`. As a result, when running the attr_encrypted test suite with Rails 7, the test fails with the following error: > lib/attr_encrypted.rb:176:in `block in attr_encrypted': undefined method `[]=' for nil:NilClass (NoMethodError) > > rake aborted! To resolve the naming clash that causes this issue, this commit renames the class method used in `attr_encrypted` from `encrypted_attributes` to `third_party_encrypted_attributes` (which is admittedly verbose 🙈). This change seems to get the test suite passing again on Rails 7. Note: I see two test failures when running the attr_encrypted test suite with Rails 7, but I see these same two failures when running the test suite with Rails 6.1.4: > 1) Failure: > ActiveRecordTest#test_attribute_was_works_when_options_for_old_encrypted_value_are_different_than_options_for_new_encrypted_value [test/active_record_test.rb:217]: > Expected: "password" > Actual: nil > > 2) Failure: > ActiveRecordTest#test_should_create_was_predicate [test/active_record_test.rb:196]: > Expected: "test@example.com" > Actual: nil
|
Help |
|
We are also using a fork with Rails 7 support, and we have the plan to migrate to standard rails encrypted attributes following this doc https://pagertree.com/2021/04/13/rails-7-attr-encrypted-migration/ |
|
Hey, @mathieujobin |
|
|
@mathieujobin thank you! |
|
Can we re-open this request and find a plan to get maintenance going again? With the onset of Rails 7 and builtin encryption, many folks are going to be hitting issues as described above. The longer term plan for most people will be to deprecate this in favor of builtin encryption, but we should maintain compatibility in the meantime. |
|
Hey guys, I'm reopening this, and sorry I've been MIA. So shortly (like weeks) after I got my then-company to offer to take this over, it became clear that the then nascent encryption feature was going to be merged into Rails 7 (the PR was expansive, had a lot of interest, and was created in apparent coordination with one of the maintainers of Rails), so it was pretty inevitable. Therefore, it seemed obvious this gem was headed for extinction and the best we could/should do was to turn it into an off-ramp for people to switch to Rails 7 when it came out. Unfortunately, while this seems like the obvious ideal end-scenario, the path to getting there became much more complicated (or else beyond my expertise to manage well). There were already a lot of things we had already known:
But on top of that, the question became if they should be prioritized at all if we were to become a simple off-ramp to or shim for the native encryption method: the work before us had apparently doubled in size. On top of that, the Rails PR was seeing enthusiastic participation raised the concern that any moves we did make around Rails 7 would be like aiming at a moving target. So long story short, there was no clear idea of how to benefit this project with the resources we had planned to offer, and it sort of lagged. In my mind I considered trying to find others who were interested, but (given how we came into the project) I was pessimistic. That said, I still should have at least posted all this and asked, and for that I apologize. I have since separated (amicably) from the company and I consequently have a bit more free time at least at the moment, and would love to create some momentum. (I actually would have replied sooner, but I wanted to clarify some things with legal first to be safe, and they haven't replied.) So.. what I still need (and greatly appreciate) though, is feedback on what our priorities are, and if there are any particular PRs or forks or any other issues that should come first. (My fear is that any one will create too many merge conflicts to take the others, and then they'll all need to be redone in this project that when we know it doesn't get much attention.) The biggest guiding factor I see (which is what I need the most help on): What type of configuration does the typical user of this gem have nowadays? Or rather, what is their upgrade path? By that I mean, if you're using the latest upstream version of To give you an idea, the current gemspec supports Rails 2. Hopefully it's understandable that unless I really have to, I don't want to go and play with the rails 2 internals to create an upgrade path for such users. But that leads me to question again who the audience is. Feedback hugely appreciated. Also, if anyone is interested in helping maintain, I would be thrilled to have you on board. (We can make t-shirts!) |
|
Thanks for hopping back on @mvastola! I think all of this makes sense and the way I see it, there isn't a huge lift needed here. I think the long term approach for the project should be to coerce folks to the Rails 7 based encryption. With that said, we do need some compatibility work to help folks (including many services at my company) to get there. As far as Ruby and Rails versions, I am of the mindset any new releases should only test/target Rails 6 and higher, as well as potentially only Ruby 2.7 and higher. (https://endoflife.date/ruby, https://endoflife.date/rails) As far as actual changes outside of testing and ruby/rails maintenance, it would seem only the following is truly needed to help unblock folks:
I am currently using this fork which seems to be working well and has minimal changes: https://github.com/InvestIMBY/attr_encrypted/tree/rails-7-0-support I am happy to help review PRs or help fixup the test suite, isn't Travis dead or at least not free anymore? |
Rails 6 is currently only supported for security issues. Do you think we could get by expecting people to go to 6.1 before upgrading here? Otherwise, seems sensible to me.
Certainly worth a try. I'll really have time to dig in this weekend, I believe.
I'm reasonably sure you're not a Russian bot, so I'll dispense with a Turing test and promote you. :-p
Travis was dead (there had only been a travis-ci.org account when I came on) and I contacted them and got us moved over and got them to give us a huge chunk of free hours. I'm pretty sure I made a branch with some tentative changes, and I think I might have pushed some travis ones (I'll find out). Travis should be publicly accessible though. One issue I do recall though was the build matrix being huge, which meant we'd need to request a lot more build minutes from Travis (they'll give them to us, but only in chunks, so you need to request each time). Thes changes might make our usage much more manageable though. Let's keep in touch. I'll give you some perms now tho. Looking to do some work this weekend. Do you use keybase? That might be effective to coordinate in general. (I had actually started a slack workspace, but running into an issue re: credentials atm, so might not be worth it.) |
|
Hey all, I thought that I'd chime in and give some context for what I was thinking when I was the maintainer. I tried to maintain as many versions of Ruby as possible because overall it didn't require too much effort and I was aware of several shops that were still on older versions of Ruby. So the amount of effort that I had to apply versus what they might have to do, it seemed like the right decision to maintain compatibility across so many versions of Ruby and Rails, even deprecated versions. However, I am aware that a lot has changed in the past few years. Many services, including Travis have dropped or limited their OSS support, so supporting so many builds is no longer practical. Moreover, the code was getting messy having to support so many older versions of Ruby and Rails. Officially deprecating support seems like a good call. I personally would do a major version bump when doing that. Insofar as the issue of Rails finally having a built in encryption feature in version 7, this gem was originally written to support other use cases, not just Rails. However, I am aware that most of the tested behavior is around Rails. That being said, if the community thinks that this gem is EOL with Rails 7, then so be it, it was fun while it lasted. Part of the reason I faltered on supporting this library is because I had learned a bunch of things the hard way about doing key management and wanted to add support for it, but also didn't see an obvious way forward on how to do it because of potential legal issues as well as compatibility issues. I wanted to make encryption nearly fool proof so that people could have good security without having to learn all the difficult lessons themselves. I don't have much time to help support this project, but I'm happy to still help where I can. |
I think a new major version is a good place to start, where we could reduce the test matrix down to Rails 6.1 and greater, and Ruby 2.7 and greater? As for Travis I can't seem to see the suite but maybe it is not public? I linked up with you on Keybase as well 👍 To your point @saghaulor I don't think we have to deprecate support of this gem in light of Rails 7 encryption, however given the lack of movement on the project the last few years I think that might just happen organically unless some folks feel passionate about reducing the backlog of issues and pull requests. |
|
Hello, I am interested in being a maintainer for this project. |
|
@AmaraFinbarrs Can you write an automate way to help projects migrating from this gem to default Rails attribute encryption? Something that would follow this doc https://pagertree.com/2021/04/13/rails-7-attr-encrypted-migration/ but make it super easy? |
|
@mathieujobin , yes I believe I should be able to. I will start looking into it now and let you know when I finish. |
|
@AmaraFinbarrs - that would be fantastic! I intend to do this in the near future. I do want to that all maintainers and contributors to |
|
if you are looking for a ruby3 compatible branch, use this one
|
|
I went ahead and incorporated the changes master...andrewfader:attr_encrypted:master and we are running this in production and it seems fine |
|
The new release should work for Rails 7 and modern Ruby versions. |
|
Needs to be tested with ruby 3.1, 3.2 and rails-edge (7.1) many gems needs to be fixed with 7.1 |
|
I am going to close this for now and folks can open issues/PRs for any further work. I do plan to add 3.1 and 3.2 to the test matrix, but also happy to review a PR. |
It's been several years since I've been engaged with this project.
Thanks to @shuber for starting this.
Thanks to @billymonk for helping maintain "way back when".
Thanks to @saghaulor for managing things since then.
None of us have had time for this in quite a while though.
So...
If you have an interest in taking over this project, please respond here. Hopefully we can find a group of interested contributors to take over.
Cheers
--Brent
The text was updated successfully, but these errors were encountered: