Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
This is a placeholder to track the port of the Security Kit module to Drupal 8. I don't see any 8.x branch for now. Maybe it would worth starting one for people to start contributing?
Comment | File | Size | Author |
---|---|---|---|
#23 | d8_response_headers-2286055-23.patch | 7.77 KB | Yury N |
Comments
Comment #1
jweowu CreditAttribution: jweowu commentedSure. Branched & pushed. Go nuts! :)
Comment #2
Anonymous (not verified) CreditAttribution: Anonymous commentedCool, I would like to help on that.
Comment #3
Anonymous (not verified) CreditAttribution: Anonymous commentedI and some friends have done some work on D8 for seckit. There is still a validation to port and 2 tests, but the other validations passed the simpletest!!!
People who contributed to this patch:
luizsgpetri
kadubr
tzambotti
Comment #4
jweowu CreditAttribution: jweowu commentedThat sounds like an excellent start. Nice work!
I don't want to get involved in the D8 side yet, but if someone would like to confirm that this is a good basis for continuing efforts, then I'll be happy to commit this to the 8.x branch, so that the subsequent patches can be smaller.
anavarre: perhaps you are in a position to provide confirmation?
n.b. I'm setting this to "Needs Review" for the above reason, but with the expectation that we'll bump it back to "Active" after the initial commit.
Comment #5
anavarre@jweowu: this port was part of a D8 hackathon (more info here) with the folks at CI&T and @typhonius and @hernani from Acquia so I trust the overall port to go in the right direction. I'll get in touch with @typhonius to discuss that more.
@luizsgpetri: I see a fatal when enabling the module:
PHP Fatal error: Call to undefined function get_t() in /var/www/html/head/modules/seckit/seckit.install on line 24
This is because get_t() is deprecated in Drupal 8.
When I remove line 24 and thus any reference to get_t() I can successfully enable the module, visit the permissions and configure pages just fine. It doesn't seem to cause any issue for now but any reference to variable_get/set/del() (more here) will have to be removed as well.
Could you please adjust that?
Comment #6
anavarreComment #7
Anonymous (not verified) CreditAttribution: Anonymous commentedThanks for the review @anavarre, I'll update the install to get rid of the old t function.
I'll double check the use of variable_X in the module but I think that should be just a few or none of them already.
Comment #8
adammaloneAdditional things I noticed with the patch initially...
Comment #9
Anonymous (not verified) CreditAttribution: Anonymous commented@anavarre and @typhonius.
Please take a look in the new patch.
Comment #10
anavarre@luizsgpetri the patch doesn't apply. At first glance it seems to be caused by a missing seckit.install file.
Also, when you post a new patch could you please post an interdiff for us to quickly review the changes?
Comment #11
Anonymous (not verified) CreditAttribution: Anonymous commented@anavarre, that is strange because I'm actually deleting the install file. It shouldn't throw any errors. It applied locally with no errors with git apply -v
. I'll create a interdiff for this patch so we can easily review the changes.
Comment #12
jeqqFix patch #9, add compatibility with Drupal 8 alpha14 and other fixies.
Comment #13
jweowu CreditAttribution: jweowu commentedCross-referencing with #2606748: [seckit] Security Kit in the Drupal 8 Contrib Porting Tracker issue queue.
Comment #14
badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commentedUpdated the patch to get it working again with rc2:
There is still plenty of work to do on this module including:
Comment #15
jweowu CreditAttribution: jweowu commentedThanks badjava.
This seems like a good time to reiterate my earlier comment:
Comment #16
badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commented@jweowu Yes, I think this is a good starting point to build on.
Comment #17
badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commentedI just discovered that this patch will need to be committed in order to use this module on sites that use Composer. Can we get this committed soon?
Comment #19
jweowu CreditAttribution: jweowu commentedCommitted.
luizsgpetri, I don't know why, but your posts in this issue are now listed as Anonymous? I could see that you're still active here, with commits in other Drupal projects, so I lifted the attribution details from one of those. I hope that's ok.
Comment #20
jweowu CreditAttribution: jweowu commentedI'm pretty sure "Needs work" is still correct here.
It feels sensible to keep development in this issue until we're at a reasonably stable point? We can just hide the redundant files as things get committed along the way.
Comment #21
timmillwoodSo now there's a
8.x-1.x
can someone make a release so we can create issues against it.I just got the following issue when running all tests against my install.
Comment #22
jweowu CreditAttribution: jweowu commentedI wasn't expecting to do that yet, but I'll defer to the people working on the code. If they'd prefer separate issues at this stage, we can do that; otherwise we'll continue to isolate the 8.x development in here.
Comment #23
Yury N CreditAttribution: Yury N as a volunteer commentedAs D8 has deprecated drupal_add_http_header() function. I've created a patch for 8.x-1.x branch that utilizes Response object.
Comment #24
badjava CreditAttribution: badjava at Metasun for Pfizer, Inc. commentedPatch applies cleanly for me and fixes the drupal_add_http_header() function deprecation issue.
With this patch we should have a functional D8 port. We need to get this patch committed and then a release cut so we can start filing individual issues against 8.x.
I am willing to help with ongoing support of this module so if someone wants to add me as a maintainer, I would be willing to take a more active role particularly with the D8 version.
Comment #25
timmillwoodI'd second @badjava to be co-maintainer.
Comment #26
dabito CreditAttribution: dabito at Globant commentedI second the move for a 8.x release so we can get this port going!
Comment #27
jweowu CreditAttribution: jweowu commentedFYI I'm on holiday for another week and a bit, and have extremely limited access to anything in the meantime. Please continue to create patches if you have improvements, and I will follow up with commits and a release once I'm back. Thanks!
Comment #29
jweowu CreditAttribution: jweowu commentedI've created a 8.x-1.x-dev release after that last commit. It should be visible shortly. Marking this issue as Fixed.
Comment #30
jweowu CreditAttribution: jweowu commented