-
Notifications
You must be signed in to change notification settings - Fork 334
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
[apps] adding G Suite app for all activity audit reports #426
Conversation
0f54afb
to
e362b65
Compare
@ryandeivert can you add support for https://developers.google.com/admin-sdk/reports/v1/guides/manage-audit-mobile? It appears to be the last remaining one per my Google foo: |
@mime-frame your google foo is excellent :) however, there are a couple of others we aren't doing. See the https://developers.google.com/admin-sdk/reports/v1/reference/activities/list Additional apps:
Do we want to support all of the above ^^ ? |
@ryandeivert - yup, it's worth supporting all of the above - you think its worth doing this in a 2nd PR or this one? There's also a SAML one too it seems: https://developers.google.com/admin-sdk/reports/v1/appendix/activity/saml |
Supporting them is a very simple change tbh so it doesn't really matter to me either way. Really annoying that googles docs say one thing one place and other thing somewhere else 🤔 |
Cool, lets get it into this PR then :) |
@mime-frame done |
* The 'auth' subdictionary within the config dict can now be accessed via the 'AppConfig.auth' property instead of requiring apps to do a dictionary lookup of 'auth' any time authentication info needs to be retrieved.
* Marking the `AppIntegration._more_to_poll` to `False` if there was nothing returned from an app's gather function * Fixing potential issue where casting auth values to a string could potentially cast dict values. This is meant to only case `unicode` to str, so checking for unicode first.
* Fixing bug in `validate-schemas` command that caused the `--test-files` flag to not work as expected
c89e6ab
to
1448066
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Can you remind me what happens to API tokens? They are encrypted in SSM, correct? The app documentation explains how the user configures the auth token, but not what happens to it
userKey='all', | ||
applicationName=self._type(), | ||
startTime=self._last_timestamp, | ||
pageToken=self._next_page_token if self._next_page_token else None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there some reason that self._next_page_token
could be an empty string? If so, you could do simply:
pageToken = self._next_page_token or None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The case of it potentially being an empty string is exactly why I added this, so good catch :). I don't know for sure if passing ''
to pageToken
would have a different effect than passing None
so I just wanted to guard against it.
I added this because I'm not sure what the default value (if any) is within the response for nextPageToken
that gets returned. It's unclear if 1) the key exists at all if there is no value, and 2) what the value would be if it does exist but is empty (is it an empty string or a json null)?
The code where this is set is here, using a .get
:
streamalert/app_integrations/apps/gsuite.py
Line 121 in 1448066
self._next_page_token = results.get('nextPageToken') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a second set of eyes on the docs would help unearth something I'm not seeing ;)
@austinbyers excellent question - the answer is yes, the API tokens/secrets are sent to SSM as a streamalert/stream_alert_cli/apps.py Lines 41 to 49 in 1448066
|
to: @javuto and @austinbyers
cc: @airbnb/streamalert-maintainers
size: large
resolves #348
Background
See #348
tl;dr - Consume all GSuite logs:
https://developers.google.com/admin-sdk/reports/v1/guides/manage-audit-admin
https://developers.google.com/admin-sdk/reports/v1/guides/manage-audit-login
https://developers.google.com/admin-sdk/reports/v1/guides/manage-audit-tokens
https://developers.google.com/admin-sdk/reports/v1/guides/manage-audit-drive
https://developers.google.com/admin-sdk/reports/v1/appendix/activity/saml
https://developers.google.com/admin-sdk/reports/v1/appendix/activity/rules
https://developers.google.com/admin-sdk/reports/v1/appendix/activity/mobile
https://developers.google.com/admin-sdk/reports/v1/appendix/activity/gplus
...
Changes
google-api-python-client
module, which has been added to therequirements.txt
(along with other dependencies) and to the 3rd party libraries that get packaged with the Apps lambda function.GSuite
apps must use a Google service account key which must be configured by an admin, along with access to theAdmin SDK
api.Other Changes
auth
property helper to theAppConfig
so theauth
subdictionary within the config dict can now be accessed via 'AppConfig.auth' instead of requiring apps to do a dictionary lookup of'auth'
any time authentication info needs to be retrieved.AppIntegration.required_auth_info
method a classmethod to the app does not need to be instantiated in the CLI to access this method.runner.py
andconfig.py
so the apps do not get instantiated when prompting the user for required authentication information.user_input
cli helper that can be used to properly validate input. Previously only a simplex is not in y
or regex comparison could be used.Bug Fixes
AppIntegration._more_to_poll
toFalse
if there was nothing returned from an app's gather function. This avoids potentially bad looping in the subclasses.unicode
to str, so checking for unicode first.validate-schemas
command that caused the--test-files
flag to not work as expectedTesting
GSuiteReportsApp
s.gsuite:reports
and added a test event for validation purposes only to test the schema.