Add access control to credentials based on group membership

.gitignore

@@ -31,6 +31,8 @@ pip-log.txt
 .tox
 nosetests.xml
 karma-test-results.xml
+coverage.xml
+package-lock.json
 
 # Translations
 *.mo

Dockerfile

@@ -6,7 +6,7 @@ WORKDIR /srv/confidant
 RUN apt-get update \
     && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
         curl ca-certificates \
-    && /usr/bin/curl -sL --fail https://deb.nodesource.com/setup_8.x | bash -
+    && /usr/bin/curl -sL --fail https://deb.nodesource.com/setup_12.x | bash -
 RUN apt-get update \
     && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \
         # For frontend
@@ -19,9 +19,6 @@ RUN apt-get update \
 
 COPY package.json /srv/confidant/
 
-RUN npm install grunt-cli && \
-    npm install
-
 COPY piptools_requirements*.txt requirements*.txt /srv/confidant/
 
 ENV PATH=/venv/bin:$PATH
@@ -32,10 +29,13 @@ RUN virtualenv /venv -ppython3 && \
 COPY .jshintrc Gruntfile.js /srv/confidant/
 COPY confidant/public /srv/confidant/confidant/public
 
-RUN node_modules/grunt-cli/bin/grunt build
-
 COPY . /srv/confidant
 
 EXPOSE 80
 
+# added this to test the saml stuff
+RUN groupadd confidantTestGroup
+RUN useradd confidant-user
+RUN usermod -a -G confidantTestGroup confidant-user
+
 CMD ["gunicorn", "confidant.wsgi:app", "--workers=2", "-k", "gevent", "--access-logfile=-", "--error-logfile=-"]

Makefile

@@ -11,6 +11,8 @@ down:
 	docker-compose down
 
 docker_build: clean
+	npm install grunt-cli && npm install
+	node_modules/grunt-cli/bin/grunt build
 	docker build -t lyft/confidant .
 
 docker_test: docker_build docker_test_unit docker_test_integration docker_test_frontend down
@@ -42,7 +44,7 @@ test_unit: clean
 	pytest --strict --junitxml=build/unit.xml --cov=confidant --cov-report=html --cov-report=xml --cov-report=term --no-cov-on-fail tests/unit
 
 test_frontend:
-	node_modules/grunt-cli/bin/grunt test
+	./node_modules/grunt-cli/bin/grunt test
 
 .PHONY: compile_deps # freeze requirements.in to requirements3.txt
 compile_deps:

confidant/authnz/__init__.py

@@ -15,12 +15,13 @@
 )
 from confidant.authnz import userauth
 
+import grp
+
 _VALIDATOR = None
 
 logger = logging.getLogger(__name__)
 user_mod = userauth.init_user_auth_class()
 
-
 def _get_validator():
     global _VALIDATOR
     if _VALIDATOR is None:
@@ -58,14 +59,27 @@ def user_is_user_type(user_type):
         return True
     return False
 
-
 def user_is_service(service):
     if not settings.USE_AUTH:
         return True
     if g.username == service:
         return True
     return False
 
+# TODO: ASK STRIPE ABOUT THIS!
+# IF GET_LOGGED_IN_USER IS AN EMAIL, WE CHECK THE GROUP MEMBERSHIP OF THE EMAIL PREFIX!
+def user_in_groups(groupnames):
+    for groupname in groupnames:
+        try:
+            group = grp.getgrnam(groupname)
+        except KeyError:
+            continue
+        user = get_logged_in_user()
+        if '@' in user:
+            user = user.split('@')[0]
+        if user in group[3]:
+            return True
+    return False
 
 def service_in_account(account):
     # We only scope to account, if an account is specified.

confidant/authnz/errors.py

@@ -1,6 +1,5 @@
 # authentication / authorization related error classes
 
-
 class UserUnknownError(Exception):
     pass
 

confidant/authnz/rbac.py

@@ -2,7 +2,7 @@
 
 from confidant import authnz
 from confidant.services import certificatemanager
-
+from confidant.models.credential import Credential
 
 def default_acl(*args, **kwargs):
     """ Default ACLs for confidant: Allow access to all resource types
@@ -25,6 +25,15 @@ def default_acl(*args, **kwargs):
     action = kwargs.get('action')
     resource_id = kwargs.get('resource_id')
     resource_kwargs = kwargs.get('kwargs')
+    if resource_type == 'credential' and resource_id is not None:
+        try:
+            cred = Credential.get(resource_id)
+        except:
+            return False
+        if len(cred.groups) and not authnz.user_in_groups(cred.groups):
+            if 'error_message_handler' in kwargs:
+                kwargs['error_message_handler']("Not a member of any of the following groups: {}".format(cred.groups))
+            return False
     if authnz.user_is_user_type('user'):
         if resource_type == 'certificate':
             return False
@@ -63,7 +72,6 @@ def default_acl(*args, **kwargs):
         # This should never happen, but paranoia wins out
         return False
 
-
 def no_acl(*args, **kwargs):
     """ Stub function that always returns true
     This function is set by settings.py by the variable ACL_MODULE

confidant/models/blind_credential.py

@@ -6,7 +6,8 @@
     NumberAttribute,
     BooleanAttribute,
     UTCDateTimeAttribute,
-    JSONAttribute
+    JSONAttribute,
+    ListAttribute
 )
 from pynamodb.indexes import GlobalSecondaryIndex, AllProjection
 
@@ -51,6 +52,7 @@ class Meta:
     modified_date = UTCDateTimeAttribute(default=datetime.now)
     modified_by = UnicodeAttribute()
     documentation = UnicodeAttribute(null=True)
+    groups = ListAttribute(default=list)
 
     def equals(self, other_cred):
         if self.name != other_cred.name:
@@ -71,4 +73,6 @@ def equals(self, other_cred):
             return False
         if self.documentation != other_cred.documentation:
             return False
+        if set(self.groups) != set(other_cred.groups):
+            return False
         return True

confidant/models/credential.py

@@ -56,6 +56,8 @@ class CredentialBase(Model):
     tags = ListAttribute(default=list)
     last_decrypted_date = UTCDateTimeAttribute(null=True)
     last_rotation_date = UTCDateTimeAttribute(null=True)
+    # if a user is not in one of the groups, cannot interact w credential
+    groups = ListAttribute(default=list)
 
 
 class Credential(CredentialBase):
@@ -82,6 +84,8 @@ def equals(self, other_cred):
             return False
         if set(self.tags) != set(other_cred.tags):
             return False
+        if set(self.groups) != set(other_cred.groups):
+            return False
         return True
 
     def diff(self, other_cred):
@@ -131,6 +135,11 @@ def diff(self, other_cred):
             'added': new.modified_date,
             'removed': old.modified_date,
         }
+        if set(old.groups) != set(new.groups):
+            diff['groups'] = {
+                'added': list(set(new.groups) - set(old.groups)),
+                'removed': list(set(old.groups) - set(new.groups)),
+            }
         return diff
 
     def _diff_dict(self, old, new):
@@ -228,6 +237,7 @@ def from_archive_credential(cls, archive_credential):
             tags=archive_credential.tags,
             last_decrypted_date=archive_credential.last_decrypted_date,
             last_rotation_date=archive_credential.last_rotation_date,
+            groups=archive_credential.groups,
         )
 
 
@@ -261,4 +271,5 @@ def from_credential(cls, credential):
             tags=credential.tags,
             last_decrypted_date=credential.last_decrypted_date,
             last_rotation_date=credential.last_rotation_date,
+            groups=credential.groups,
         )

confidant/public/modules/history/controllers/CredentialHistoryCtrl.js

@@ -54,12 +54,24 @@
                             $scope.noDiff = true;
                         }
                     }, function(res) {
-                        $scope.getError = res.data.error;
+                        if (res.status === 500) {
+                            $scope.getError = 'Unexpected server error.';
+                            $log.error(res);
+                        } else {
+                            $scope.getError = res.data.error;
+                        }
                     });
                 }
                 if ($scope.currentRevision === $scope.credentialRevision) {
                     $scope.isCurrentRevision = true;
                 }
+            }, function(res) {
+                if (res.status === 500) {
+                    $scope.getError = 'Unexpected server error.';
+                    $log.error(res);
+                } else {
+                    $scope.getError = res.data.error;
+                }
             });
 
             $scope.shouldDisplayList = function(value) {

confidant/public/modules/resources/controllers/CredentialDetailsCtrl.js

@@ -85,19 +85,25 @@
             if ($scope.credentialId) {
                 CredentialServices.get({'id': $scope.credentialId}).$promise.then(function(credentialServices) {
                     $scope.credentialServices = credentialServices.services;
-                });
-
-                Credential.get({'id': $scope.credentialId, 'metadata_only': true}).$promise.then(function(credential) {
-                    $scope.shown = false;
-                    populateCredential(credential);
+                    Credential.get({'id': $scope.credentialId, 'metadata_only': true}).$promise.then(function(credential) {
+                        $scope.shown = false;
+                        populateCredential(credential);
+                        }, function(res) {
+                            if (res.status === 500) {
+                                $scope.getError = 'Unexpected server error.';
+                                $log.error(res);
+                            } else {
+                                $scope.getError = res.data.error;
+                            }
+                            deferred.reject();
+                        });
                 }, function(res) {
                     if (res.status === 500) {
                         $scope.getError = 'Unexpected server error.';
                         $log.error(res);
                     } else {
                         $scope.getError = res.data.error;
                     }
-                    deferred.reject();
                 });
             } else {
                 // A new credential is being created
@@ -106,7 +112,8 @@
                     enabled: true,
                     credentialPairs: [{'key': '', 'value': ''}],
                     mungedMetadata: [],
-                    mungedTags: []
+                    mungedTags: [],
+                    groups: [],
                 };
                 credentialCopy = angular.copy($scope.credential);
                 $scope.shown = true;
@@ -221,6 +228,7 @@
                 _credential.name = $scope.credential.name;
                 _credential.enabled = $scope.credential.enabled;
                 _credential.documentation = $scope.credential.documentation;
+                _credential.groups = [];
                 _credential.credential_pairs = {};
                 _credential.metadata = {};
                 _credential.tags = [];
@@ -253,6 +261,14 @@
                     }
                     _credential.metadata[metadataItem.key] = metadataItem.value;
                 }
+                // Remove whitespace from group names
+                var unmungedGroups = $scope.credential.groups.split(',');
+                for (i = 0; i < unmungedGroups.length; i++) {
+                    var grp = unmungedGroups[i].trim();
+                    if (grp.length > 0) {
+                        _credential.groups.push(grp);
+                    }
+                }
                 for (i = $scope.credential.mungedTags.length; i--;) {
                     var tagItem = $scope.credential.mungedTags[i];
                     if (tagItem.isDeleted) {

confidant/public/modules/resources/views/credential-details.html

@@ -93,6 +93,10 @@
         <button type="button" ng-disabled="editableForm.$waiting" ng-click="addCredentialPair()" class="btn pull-right">Add row</button>
       </span>
     </div>
+    <div class="form-group">
+      <label for="credentialGroupListInput">Credential Groups</label>
+      <span editable-text="credential.groups" id="credentialGroupListInput" style="color:#333333">{{ credential.groups.join(",") || 'Not set' }}</span>
+    </div>
     <div class="form-group">
       <label for="credentialMetadata">Credential Metadata</label>
       <div class="well well-sm" id="credentialMetadata">

confidant/routes/certificates.py

@@ -248,7 +248,6 @@ def list_cas():
        GET /v1/cas
 
     **Example response**:
-
     .. sourcecode:: http
 
        HTTP/1.1 200 OK
@@ -310,7 +309,6 @@ def get_ca(ca):
     :type ca: str
 
     **Example response**:
-
     .. sourcecode:: http
 
        HTTP/1.1 200 OK