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

check network access for Request an Account page #1476

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

RabiaSajjad
Copy link
Member

No description provided.

@RabiaSajjad RabiaSajjad requested a review from wardi May 17, 2024 17:30
ckanext/canada/view.py Outdated Show resolved Hide resolved
ckanext/canada/auth.py Outdated Show resolved Hide resolved
ckanext/canada/auth.py Outdated Show resolved Hide resolved
ckanext/canada/helpers.py Outdated Show resolved Hide resolved
ckanext/canada/plugins.py Outdated Show resolved Hide resolved
ckanext/canada/plugins.py Outdated Show resolved Hide resolved
ckanext/canada/tests/test_webforms.py Outdated Show resolved Hide resolved
ckanext/canada/helpers.py Outdated Show resolved Hide resolved
ckanext/canada/helpers.py Outdated Show resolved Hide resolved
ckanext/canada/helpers.py Outdated Show resolved Hide resolved
- Fixed merge conflicts.
- Updated pytests for py3.
@JVickery-TBS
Copy link
Contributor

@RabiaSajjad I rebased this now as python3 code is in Master branch. I updated the pytests for python3 too. So we will see if those are successful.

'api.action', # change if need to narrow down the scope
]
if blueprint in restricted_blueprints and not helpers.registry_network_access():
return abort(403)
Copy link
Member

Choose a reason for hiding this comment

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

might want to log failed access attempts so we can report on them

Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea. Do you think we need a separate log handler for "Registry Access"? As we will need to also log failed login attempts and all login sessions.

We could have a new log handler for ckanext.canada.user_access to go to its own log file ckan_registry_access.log.

Or should we just keep it in the normal logs and have Log Analytics handle all this stuff @wardi ?

Copy link
Member

Choose a reason for hiding this comment

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

@JVickery-TBS I don't have a strong opinion either way. As long as the data can be pulled out of Log Analytics it shouldn't matter.

Copy link
Member Author

Choose a reason for hiding this comment

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

@wardi and @JVickery-TBS let's implement logging as a separate feature for both registry network access and login access. It is one of the requirements given to us by imtd/security. I would prefer to implement it as a separate log handler so that it is easier for us to investigate any issues reported by helpdesk.

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.

3 participants