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

Fix broken auditlog browsing and auditlog more things #1654

Merged
merged 35 commits into from Mar 7, 2018

Conversation

jmbredal
Copy link
Collaborator

@jmbredal jmbredal commented Jan 17, 2018

Auditlog did not display log entries properly. There were inconsitencies between getting data from context or api.

Changes done:

  • Add log entries on crud of profiles
  • Add log entries on crud of devices
  • Add log entries when logging in and out

DataTable

  • Use api only for data
  • Add filters, sorting and searching
  • Add DataTable to ipdevinfo (new tab)
  • Fix DataTable for portadmin
  • Fix DataTable main tool
  • Add DataTable to user info (list and details)

@lunkwill42 lunkwill42 changed the title Auditlog enhancement Fix broken auditlog browsing and auditlog more things Mar 5, 2018
Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

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

I think this still needs some fixing - adding a tasklist comment with my feedback

@lunkwill42
Copy link
Member

lunkwill42 commented Mar 7, 2018

This is my general feedback after giving this a quick look-over:

  • I reloaded an auditlog page after switching to this branch, and got an error from /auditlog/actor/account/:

    Exception Type: TemplateDoesNotExist at /auditlog/actor/account/
    Exception Value: auditlog/logentry_list.html
    

    Not sure that this page is reachable any longer with your changes - in that case, urls.py and the defined views may need some pruning.

  • I added a user to the Adminstrators group, and the auditlog reads "mvold added group NAV Administrators to foo". The grammar doesn't sound quite right: I didn't add a group to a user, I added the user to the group...

  • There seems to be some inconsistensies in the verbs that are being used across NAV. Portadmin uses "verbs" with spaces separating multiple words, while other systems appear to use verbs with dashes separating words...

  • We should maybe clean the UI: "Audit log" is two words in English, not one (and the camelcasing doesn't look pretty either)

  • Audit log entries concering config changes to ports do not appear in the device's Audig log tab. Won't users find that strange?

  • I'm not sure I like having the audit log tab in ipdevinfo at all, when I think about it. It makes it harder (given NAV's current auth system) to limit who is allowed to set audit logs. Can't the Audit log subsystem accept a netbox idenficator to filter in its URL, and then we can just link to this? It still requires one click.

  • When changing SNMP communities, both the old and new values are logged in plaintext in the auditlog. We take care in most places to not print the communities unless you are editing them in SeedDB - should we not censor this?

  • If we are audit logging changes to user objects, should we not also audit log changes to API tokens as well?

Some of these could very well be separate PRs, but we should discuss which one's we need to do something about before we merge this PR.

@jmbredal
Copy link
Collaborator Author

jmbredal commented Mar 7, 2018

I consider the rest bigger tasks that we can do at a later time.

@lunkwill42
Copy link
Member

Great. Added #1675, #1676 and #1677 for followup. Waiting for CI results before merging.

@lunkwill42 lunkwill42 self-assigned this Mar 7, 2018
@lunkwill42 lunkwill42 added this to the 4.8.4 milestone Mar 7, 2018
@lunkwill42 lunkwill42 merged commit d9bc125 into Uninett:4.8.x Mar 7, 2018
@jmbredal jmbredal deleted the auditlog-enhancement branch April 18, 2018 09:01
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.

None yet

2 participants