strongdm / accessbot Goto Github PK

Describe the bug
Currently the status of each command is handled using a property of the chatbot class. This could run into race conditions, making the bot thread unsafe.

To Reproduce
Send several access requests inside of the admin timeout period.

Additional context
Ideally use isolated callback functions that don't rely on the object state. A TODO comment was added to the code.

I think the 1 minute delay was added because temporary grants are required to be created in the future, but we recently added a 1 minute grace period, so we should be able to remove the delay on this line now:

Is your feature request related to a problem? Please describe.
Currently accessbot integrates with Slack via RTM, which is only supported by classic apps. Slack encourages users to move to the Events API or Socket Mode.

When using Events API, the integration complexity behind firewalls or corporate networks would increase significantly, because the bot would need to expose an endpoint for receiving Slack events.

In order to support new Slack apps and keep integration complexity low, it's recommended to use Socket Mode.

Describe the solution you'd like
Support Slack integration via Socket Mode

Describe alternatives you've considered
I found on the errbot gitter group a reference to the following "experimental" backends:

• https://github.com/nzlosh/err-backend-slackv3
• https://github.com/attakei-lab/errbot-slack-bolt-backend

Currently, after enabling auto approval number of requests from users are expected to grow and users may end up requests more number of times than they might actually nee.

The idea here is not to promote the auto approval request from users unless it is absolutely necessary. Hence, we would like to limit auto approval requests for a given period of time.

Example - say user can grant him/her self an automatic access 10 times a week before falling back to default manual approval workflow.

Is your feature request related to a problem? Please describe.
Auto approval is binary right now, all or nothing. It would be ideal to allow auto-approve only for certain resources.

Describe the solution you'd like
Allow auto approve for tagged resources. Ideally, if the resource has a tag (e.g. auto-approve), no matter which value, it should be a candidate for auto approvals. In case the resources doesn't have the configured tag, the system should follow the default flow.

Additional context
When auto-approve-all is enabled, this option should be ignored and just auto-approve everything.

In a Cloud Native App, environment variables should be injected to the container in execution, rather than backed into it.

Is your feature request related to a problem? Please describe.
Currently, all logs are saved into a file named errbot.log. If acessbot runs in a container, all logs should be sent to the stdout.

Describe the solution you'd like
When SDM_DOCKERIZED=true send logs to stdout

Additional context
Not sure yet how errbot handles this type of scenarios, but there are 2 important things to explore:

1. errbot uses standard Python logging library
2. If errbot ignores logging config, then symlinks for the errbot.log could do the trick

Is your feature request related to a problem? Please describe.
Some organizations use different emails for SDM and Slack. For those cases, we might want to use custom fields for alternate emails

Describe the solution you'd like
Add an environment variable that can be set for specifying a custom field to be used as the user email.

Additional context
User need registered in ticket 3064

Is your feature request related to a problem? Please describe.
When an access request is stale and cleaned, the poller notifies directly requester and admins. When messages come from a channel, it would be ideal to send all messages to the originator channel so there's only one available "log".

Describe the solution you'd like
Send stale cleaner messages to originator channels. Use message.room variable when present to notify requester and admins.

Describe the bug
Right now, if the tag hide-resource is set to anything, the resource will be hidden. ie: even if it's set to false

To Reproduce
Set tag hide-resource to false, the resource will be hidden from accessbot

Expected behavior
If hide-resource isn't true, the resource shouldn't be hidden.

Is your feature request related to a problem? Please describe.
During our demo, JFrog asked if we could replace "admin1, admin2" with an "admin channel name". They said that with different on-call schedules, with different teams, they won't always know who the "current" admins are.

Describe the solution you'd like
Add an option to use an approval channel. This would be a private Slack channel with strict access rules (on the Slack side). Any "yes code" reply made in that channel would serve to approve a request.

Describe alternatives you've considered
None.

Is your feature request related to a problem? Please describe.
By default all access requests need approval from an admin user, it would be ideal to switch on or off this behavior, allowing users to receive approvals automatically for all their requests without admin validation.

Describe the solution you'd like
A feature flag that admins can turn on via environment variables for enabling the auto-approve-all capability.

Describe alternatives you've considered
For further releases, include a new feature flag: auto-approve-by-tag, or similar, that automatically approves certain tagged resources.

Additional context
Some clients already manifested interest in this functionality

Is your feature request related to a problem? Please describe.
The list of available resources is composed of all resources configured in an account. Resources can be made "unavailable" using the config HIDE_RESOURCE_TAG. However, this method can be inconvenient when there are many resources or clients are used to use roles.

Describe the solution you'd like
Add a new plugin config named CONTROL_RESOURCES_ROLE_NAME where the value indicates a valid role name to be used for getting all available resources (default = None).

Describe alternatives you've considered
See this script: pager_duty_ROLE.py

Describe the bug
We don't use quotes in the YAML, this means that an overrride email address can have (invisible) spaces. We should check for this; or strip spaces in front/back; or confirm/recommend use of quotes here.

To Reproduce

Enter this in YAML:

"SDM_SENDER_EMAIL_OVERRIDE=[email protected] "

(quotes added for visibility).

Expected behavior
Email lookup should work.

Actual
Fails.

Is your feature request related to a problem? Please describe.

Ideally for automated approval workflow on production, we would like grant users permission to self serve themselves by issuing command access to data-source using chatbot.

There is another way we could approach this independently without involving slack is via some script (runs regularly) against PagerDuty API. However, that will probably check the on call rotation and grant them permission.

We ideally don't want users to have access to data sources (DB) even when they are ON CALL. The request has to come explicitly by the user and can be approved automatically given that the request is coming from ON CALL user. hope this makes sense.

Add a bot config gif to the README file so that users can better understand the process involved in updating bot configuration via plugin config

Is your feature request related to a problem? Please describe.
Corvesta has said that in some cases, users need to get access to multiple resources at once. Requesting access one at a time is tiresome, even with auto-approval. They ask if we can allow users to request Role assignment, i.e. assign me to role OnCall2021

Describe the solution you'd like
Add a new command, e.g. assign me to role

Describe alternatives you've considered
Dynamic Grants might be an alternative, after they are GA.

Is your feature request related to a problem? Please describe.
Imagine a system-wide issue occurs at night, when an on-call Engineer needs access to an SDM resource to solve a system-down problem. It might be good to have a "failsafe" option, where any user gets automatic access during a defined window.

Describe the solution you'd like
Add a failsafe option, as a binary option. If TRUE, a user access request will be auto-approved if it occurs during a defined time window.

Describe alternatives you've considered
Might want to limit this to certain users, perhaps defined in a list as "on-call users". User would need to be in that list, and match the time window, to get auto-approval.

Another option could be an "on-call" admin -- ping different folks for approval depending on time of day.

Additional context
Arguably, this reduces security. But users still must a) belong to the Slack workspace, b) have access to the channel, and c) all grants and activities are audited in SDM.

Is your feature request related to a problem? Please describe.
Although one can observe who executed a command in DEBUG level, that information is not present when using the default logging level (INFO). It's fundamental to easily get that information from the logs at all times.

Describe the solution you'd like
Include who entered a command in INFO logs.

Describe alternatives you've considered
That info can be extracted from message.frm

Is your feature request related to a problem? Please describe.
Today, users who are denied access get a generic message to "contact their admin". We know who the admin is, why not tell the requesting user?

Describe the solution you'd like
Add the admin1 variable text to the denial message.

Describe alternatives you've considered
If we implement more complex admin rules, i.e. based on the user type, would need to revisit this.

Is your feature request related to a problem? Please describe.
Currently, the temporal grant timeout is fixed to 1 hour. Certain clients would like to change that behavior via plugin config.

Describe the solution you'd like
Include "temporal grant timeout" as a plugin configuration with a default value of 60 minutes.

Describe alternatives you've considered
Ideally the new variable should be configurable in minutes.

There's a technical debt pending, the approve request regex was improved and core plugins were removed via hotfixes, but tests were not added. Pay the debt 😄

Is your feature request related to a problem? Please describe.
Today I was testing Accessbot, and could not run show available resources command due to #71, meaning I could not be sure of which resources were requestable. I just copied some resource names from our UI. One kept giving me the error Invalid resource name. Only after 5 of these did I think to run sdm admin servers list -e to view tags and realize that I had tagged this resource to hide it last month!

I think this message is misleading, as a user may know a resource name well, think I KNOW this name is right! and get stuck.

Describe the solution you'd like
Let's change to something like Access to this resource not available via bot. Please see your strongDM admins.

The code is inconsistent in terms of naming conventions, e.g. not using name mangling convention for internal details on classes.

See: The Meaning of Underscores in Python

Describe the bug
Running show available resources in an org with k8s beta resources gives this error:

List resources failed: ("unknown resource type 'kubernetesauthenticatingproxy'", 2)

I think we've seen this recently with other beta resources, and it's related to not having/running our latest Python SDK.

To Reproduce
I used the latest image from here.
In your test org, add a k8s beta resource.
Run show available resources in Slack.

Expected behavior
You see resources.

Is your feature request related to a problem? Please describe.
When the admin approves a request, the current behavior is: wait until the ADMIN_TIMEOUT finishes to grant access and inform the user and admin about the approval. This behavior works ok for short timeouts, but it can be a pain when timeouts are +5min.

Describe the solution you'd like
When the one of the admins approves a request, the grant should occur immediately and the admins and user notified.

In the CallbackMessageHandler grant access and notify, only leave logic for notifying approval denial in AccessHandler

Errbot already provides several options for customizing the help command, specially interesting:

• HIDE_RESTRICTED_COMMANDS
• HIDE_RESTRICTED_ACCESS
• DIVERT_TO_PRIVATE

Ideally, AccessBot should use help plugin and include all documentation directly in the accessbot.py file.

Add a health check gif to the README file so that users can better see how a healthy installation looks like

Is your feature request related to a problem? Please describe.
As an admin I want to control the roles that my users can request access to so that I can control resources via groups.

For example:

• Grant user X all resources under role DEV
• Grant user X all resources under role PROD

A user should be able to request access from 1+ role(s)

Describe the solution you'd like
Create a new environment variable: SDM_USER_ROLES_TAG. The value indicates a variable that can be read in order to control the roles a user can request access to. IMPORTANT: Support multiple values, e.g. using comma separated values.

For example: When SDM_USER_ROLES_TAG=sdm-roles

$ sdm admin users list
User ID                First Name     Last Name     Email                            Tags
a-xxx                  Firstname1     Lastname1     [email protected]
a-yyy                  Firstname2     Lastname2     [email protected]
$ sdm admin users update --email [email protected] --tags 'sdm-roles="dev,prod"'
$ sdm admin users list
User ID                First Name     Last Name     Email                              Tags
a-xxx                  Firstname1     Lastname1     [email protected]          sdm-roles="dev,prod"
a-yyy                  Firstname2     Lastname2     [email protected]
$ sdm admin users update --email [email protected] --delete-tags 'sdm-roles'

Additional context
Some of our clients want to control environments a user can access. Usually, environment resources can be represented with SDM roles.

Is your refactoring related to a problem? Please describe.
The grantbot class has several pending improvements in order to make it easier to read.

• Name. The project name changed from grantbot to accessbot, but that's not reflected in the code yet
• Helper methods. The class is the core of the project, and all the logic there makes it hard to read
• Collaborators instantiation. Singleton methods implementation on the different collaborator files is ugly and can be avoided.

Describe the solution you'd like

• Name. Change the name of the project and validate tests
• Helper methods. Create separate class for each different command
• Collaborators instantiation. Remove singletons and transfer responsibility to the accessbot class

Is your feature request related to a problem? Please describe.
There's no option for knowing the current version of the bot at execution time, ideally there should be a flag in config providing that information. IMPORTANT: Disallow version change through plugin config.

Describe the solution you'd like
As an admin I want to see the bot version when running plugin config AccessBot so that my team can be better supported.

In order to support this, we might want to create docker images and releases.

For managing versions, we can use semantic versioning. Take a look at: https://github.com/camposer/versioneer-example

Describe alternatives you've considered
Use errbot for showing the plugin version, but apparently there's no builtin capability that.

The current implementation passes a full list of "instance properties" on every execution. Ideally, if we keep the class, only what's changing in every execution should be passed to execute(), otherwise remove the class and just implement a module function.

Is your feature request related to a problem? Please describe.
Today, if a user already has access to resource X, and requests access to it, we ask the admin for approval, try the grant, then it fails. It would be nice not to bother the admin in this case -- just tell the user right away.

Describe the solution you'd like
We can't do a "dry run" on the grant in the SDK today. So we should probably check the user's current grants with the named resource.

Is your feature request related to a problem? Please describe.
Depending on a single admin risks many denials, just because a single user is busy or out of office.

Describe the solution you'd like
We should support at least a second admin.

Describe alternatives you've considered
Another option could be a user-defined list. The bot could read the list and perform "length" requests until it gets an approval.

Is your feature request related to a problem? Please describe.
Corvesta (client) would like the workflow for adding roles to be automated like it is for accessing datasources.

Describe the solution you'd like
a user can ask access bot for access to a role and it will be granted automatically

Describe alternatives you've considered
n/a - manual method is currently supported for roles

Additional context
n/a

IMPORTANT: Remember to select a label and project

Is your feature request related to a problem? Please describe.
Show a list of available resources, so users know what is available -- can can copy/paste, to minimize misspelling resource names.

Describe the solution you'd like
Can we list all resources with one call? Or do we have to go by type, e.g. datasources, clusters, etc.

Describe alternatives you've considered
Could be a problem for orgs with large numbers of resources.

Is your feature request related to a problem? Please describe.
Larger, production scenarios involve different hierarchies. One might want to have admin1 approve requests for all support users, but admin2 for all Engineering users.

Describe the solution you'd like
I don't have a clear idea of how this would be implemented. Perhaps a dictionary that maps users to admins. Upon request, use the requestor's email to find the correct admin?

Is your feature request related to a problem? Please describe.
The access to command is the key component of the bot. It was originally scripted without tests and separation of concerns. Ideally we should change that.

Describe the solution you'd like
Move command logic to a service, add unit tests to it (mocking SDM API SDK) and errbot integration tests for at least one happy and sad path.

Additional context
Take a look at the tests implemented for help command. For mocking the SdmClient you could use patch from unittest.mock. See: https://gist.github.com/mmasashi/d45d2fc5f32fba9ae2b91506976099a8

As a developer I want a CI tool to run all tests when pushing to main branch so that we can ensure the project is always in a healthy state

Similar to #6 but for access to role role-name (aka assign role)

Is your feature request related to a problem? Please describe.
As an admin I want to change bot configurations (properties) via slack so that it's not necessary to restart the bot every time I need to change its behavior

Describe the solution you'd like
Introduce three new commands:

• show config. Shows bot configuration (excluding credentials - slack, sdm)
• set config key value. Sets the value for a specific config
• reset config: Sets config back to its original state

Describe alternatives you've considered
Can create separate issues for this feature request

Additional context
Add any other context or screenshots about the feature request here.

Describe the bug
Running show available resources for an Org that has beta features can throw this error:

Computer says nooo. See logs for details:
'NoneType' object has no attribute 'name'

I saw this before when AWS Cloud was in beta. Now it's because of the new k8s stuff -- when I removed those resoruces from my org, the command completed fine.

To Reproduce
Run Accessbot under an org that has at least one k8s-impersonation resource.

Expected behavior
Command works.

This was with latest Accessbot code.

Is your feature request related to a problem? Please describe.

How can we limit the datasources the users have access to? I don’t want them to be able to request an admin datasource for example.

Describe the solution you'd like
Corvesta plan to use the bot without approval -- otherwise, the admin could simply reject such requests. In either scenario, however, this could still be useful, i.e. to reduce traffic to approvers, or if we implement the "after hours" feature to bypass approval.

Best approach might be a blacklist that admins can configure -- the bot would check this list after validating the request.

Describe alternatives you've considered
Getting more complex, but we could have a similar file with groups of users that dictate what they are "allowed" to request.

Additional context
Add any other context or screenshots about the feature request here.

Is your feature request related to a problem? Please describe.
Today, you can run !help to use built-in Errbot help commands. But this shows a lot of less than helpful errbot commands.

Describe the solution you'd like
We could override this, or write a (non-prefixed) help command. Maybe pin this command ref to the channel?

Describe the bug
When the bot is waiting for a manual approval, if an admin command is executed (e.g. plugin config) the bot hangs until the ADMIN_TIMEOUT expires.

To Reproduce
Steps to reproduce the behavior:

1. Enter access to resource-name
2. Enter plugin config Accessbot
3. Enter any other command (it wouldn't work)

Screenshots

Additional context
Already tried:

BOT_ASYNC = True
BOT_ASYNC_POOLSIZE = 10

According to this issue it's better to use the scheduling capabilities of errbot

Describe the bug
Commands types in bold font are ignored

To Reproduce
Commands types in bold font are ignored

Expected behavior
Only the text should matter.

Is your feature request related to a problem? Please describe.
If you don't spell a resource name exactly, you get denied. Fuzzy matching would allow near-misses to be corrected.

Describe the solution you'd like
Maintain a list of all resources -- would have to be per-session at least, to catch newly created resources -- to match against.

Describe the bug
The current slack-manifest.yaml configuration is missing the OAuth Scopes for Writing, please add them.

Here's the file you can use for reference:

_metadata:
  major_version: 1
  minor_version: 1
display_information:
  name: accessbotDavid
  description: AccessBot using Bolt
  background_color: "#2c2d30"
features:
  app_home:
    home_tab_enabled: false
    messages_tab_enabled: true
    messages_tab_read_only_enabled: false
  bot_user:
    display_name: accessbot_bolt
    always_online: false
oauth_config:
  scopes:
    bot:
      - channels:history
      - channels:read
      - chat:write
      - groups:history
      - groups:write
      - im:history
      - im:write
      - incoming-webhook
      - mpim:write
      - reactions:write
      - users:read
      - users:read.email
      - groups:read
settings:
  event_subscriptions:
    bot_events:
      - message.channels
      - message.groups
      - message.im
  interactivity:
    is_enabled: true
  org_deploy_enabled: false
  socket_mode_enabled: true
  token_rotation_enabled: false

Add a gif to the README with a simple main commands tutorial

Is your feature request related to a problem? Please describe.
Today, a user who has their request denied gets a generic message. Allow the admin to type in a reason.

Describe the solution you'd like
The callback method today just listens for "yes". We might change this, or add a new method to listen for "no", then capture the following line, if it exists.

Describe alternatives you've considered
Might want to improve the callback approach and listen for any reply, perhaps using the thread id?