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

Added Audit logging to new user creation #17852

Merged
merged 3 commits into from
Aug 24, 2018

Conversation

juliancheal
Copy link
Member

@juliancheal juliancheal commented Aug 13, 2018

Fixes BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1602136

Raises an audit event when new external logins are auto-created for the first time.

Creates the following audit message:

User creation successful for User: Bob Builderson with ID: bob

@juliancheal juliancheal changed the title [WIP] Added Audit logging to new user creation Added Audit logging to new user creation Aug 14, 2018
@miq-bot miq-bot removed the wip label Aug 14, 2018
@juliancheal
Copy link
Member Author

juliancheal commented Aug 22, 2018

@jvlcek @Fryguy I've got the tests passing for http auth https://github.com/ManageIQ/manageiq/pull/17852/files#diff-d367fc51a358614f7b1d4e0dced7d0f4, but what I don't understand in my tests is my user creation audits need to be in lowercase, whereas the other tests are mixed case.

example

expect(AuditEvent).to receive(:success).with(
  :event   => 'authorize',
  :userid  => 'bob',
  :message => "User creation successful for User: Bob Builderson with ID: bob@example.com",
)
expect(AuditEvent).to receive(:success).with(
  :event   => 'authenticate_httpd',
  :userid  => 'bOb',
  :message => "User bob successfully validated by External httpd",
)

@jvlcek
Copy link
Member

jvlcek commented Aug 22, 2018

@juliancheal The tests were updated to include mixed case to ensure the auth code correctly handles mixed case. I'm not sure why your tests need the userid in mixed case but I'd be glad to pair up and try to figure it you, if you'd like.

@juliancheal
Copy link
Member Author

@jvlcek let's meet after standup.

@jvlcek
Copy link
Member

jvlcek commented Aug 23, 2018

👍 LGTM

Userid was being stored in audit event, before being downcased.
Changing the order of those two lines now makes the tests make more sense.
@miq-bot
Copy link
Member

miq-bot commented Aug 23, 2018

Checked commits juliancheal/manageiq@2012beb~...53f2be3 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 0 offenses detected
Everything looks fine. 🍪

@juliancheal
Copy link
Member Author

@gtanzillo @Fryguy We ok to merge?

@juliancheal
Copy link
Member Author

@miq-bot add_label gaprindashvili/yes

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gtanzillo gtanzillo added this to the Sprint 93 Ending Aug 27, 2018 milestone Aug 24, 2018
@gtanzillo gtanzillo merged commit e8405ff into ManageIQ:master Aug 24, 2018
juliancheal pushed a commit to juliancheal/manageiq that referenced this pull request Oct 3, 2018
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.

4 participants