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

Feature/initial code #1

Open
wants to merge 39 commits into
base: main
Choose a base branch
from
Open

Feature/initial code #1

wants to merge 39 commits into from

Conversation

A-Souhei
Copy link

@A-Souhei A-Souhei commented Feb 3, 2025

Description

Initial code for this new extension.
Features:

  • Adds OWASP headers
  • Removes image_url capability in user/group/organization creation and update

Added tests for them.

The values on the picture are changed in order to test if the variables are read.
image

image

Relates https://github.com/fjelltopp/who-afro-ckan/pull/622
Relates https://github.com/fjelltopp/fjelltopp-infrastructure/pull/349
Closes https://github.com/fjelltopp/who-afro-ckan/issues/609
Closes https://github.com/fjelltopp/who-afro-ckan/issues/612

Checklist

Put an x in the boxes that apply to this pull request (you can also fill these out after opening the pull request).
You may not need to check all boxes.

  • The GitHub ticket for this issue has been updated to "Ready to Review" or equivalent.
  • I have developed these changes in discussion with the appropriate project manager.
  • My code follows the general Fjelltopp documentation (see Confluence).
  • I have made corresponding changes to the Fjelltopp documentation (see Confluence).
  • I have rebased this branch with master.
  • New dependency changes have been committed.
  • I have added automated tests that prove my fix is effective or that my feature works.
  • New and existing tests pass locally with my changes.
  • My changes generate no new warnings.
  • I have performed a self-review of my own code.
  • I have assigned at least one reviewer.
  • I have assigned at least one label to this PR: "patch", "minor", "major".

@A-Souhei A-Souhei self-assigned this Feb 3, 2025
@A-Souhei A-Souhei added the enhancement New feature or request label Feb 3, 2025
@A-Souhei A-Souhei added good first issue Good for newcomers major labels Feb 3, 2025
@cooper667
Copy link

I wonder if this would be the place to handle the caching headers @ChasNelson1990 ?

Ideally we want to be able to set them by file extension I think, looks like there's some overlap with this?

@A-Souhei A-Souhei marked this pull request as ready for review February 3, 2025 10:11
@A-Souhei
Copy link
Author

A-Souhei commented Feb 3, 2025

overlap

@cooper667 I do not understand, could you explain please?

@A-Souhei
Copy link
Author

A-Souhei commented Feb 3, 2025

I will fix after the reviews!

@cooper667
Copy link

overlap

@cooper667 I do not understand, could you explain please?

Not a problem for this PR! It's more just me wondering whether the stuff im going to work on belongs in this extension. It's not quite security but it does involve changing the headers so may make sense logically

Copy link
Member

@jonathansberry jonathansberry left a comment

Choose a reason for hiding this comment

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

Hi Toavina, thanks for getting this together. Good progress. There are few things concerning me slightly, and af ew other hints and tips around writing good CKAN code. Hopefully it is helpful!

ckanext/fjelltopp_security/i18n/.gitignore Outdated Show resolved Hide resolved
ckanext/fjelltopp_security/assets/.gitignore Outdated Show resolved Hide resolved
ckanext/fjelltopp_security/public/.gitignore Outdated Show resolved Hide resolved
ckanext/fjelltopp_security/templates/.gitignore Outdated Show resolved Hide resolved
ckanext/fjelltopp_security/plugin.py Outdated Show resolved Hide resolved
ckanext/fjelltopp_security/plugin.py Outdated Show resolved Hide resolved
ckanext/fjelltopp_security/plugin.py Outdated Show resolved Hide resolved
ckanext/fjelltopp_security/tests/conftest.py Outdated Show resolved Hide resolved
ckanext/fjelltopp_security/tests/test_groups.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants