diff --git a/config.py b/config.py index 369c850c2..f91637992 100644 --- a/config.py +++ b/config.py @@ -527,3 +527,7 @@ class DefaultConfig(ImmutableConfig): # Defines the number of successive internal errors of a build trigger's build before the # trigger is automatically disabled. SUCCESSIVE_TRIGGER_INTERNAL_ERROR_DISABLE_THRESHOLD = 5 + + # Defines the delay required (in seconds) before the last_accessed field of a user/robot or access + # token will be updated after the previous update. + LAST_ACCESSED_UPDATE_THRESHOLD_S = 60 diff --git a/data/database.py b/data/database.py index ea1353830..422275f84 100644 --- a/data/database.py +++ b/data/database.py @@ -440,8 +440,8 @@ class User(BaseModel): location = CharField(null=True) maximum_queued_builds_count = IntegerField(null=True) - creation_date = DateTimeField(default=datetime.utcnow, null=True) + last_accessed = DateTimeField(null=True, index=True) def delete_instance(self, recursive=False, delete_nullable=False): # If we are deleting a robot account, only execute the subset of queries necessary. diff --git a/data/migrations/versions/224ce4c72c2f_add_last_accessed_field_to_user_table.py b/data/migrations/versions/224ce4c72c2f_add_last_accessed_field_to_user_table.py new file mode 100644 index 000000000..06326c566 --- /dev/null +++ b/data/migrations/versions/224ce4c72c2f_add_last_accessed_field_to_user_table.py @@ -0,0 +1,28 @@ +"""Add last_accessed field to User table + +Revision ID: 224ce4c72c2f +Revises: b547bc139ad8 +Create Date: 2018-03-12 22:44:07.070490 + +""" + +# revision identifiers, used by Alembic. +revision = '224ce4c72c2f' +down_revision = 'b547bc139ad8' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(tables): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('user', sa.Column('last_accessed', sa.DateTime(), nullable=True)) + op.create_index('user_last_accessed', 'user', ['last_accessed'], unique=False) + # ### end Alembic commands ### + + +def downgrade(tables): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index('user_last_accessed', table_name='user') + op.drop_column('user', 'last_accessed') + # ### end Alembic commands ### diff --git a/data/model/_basequery.py b/data/model/_basequery.py index 28b0b1952..571b8ac9b 100644 --- a/data/model/_basequery.py +++ b/data/model/_basequery.py @@ -1,11 +1,17 @@ -from peewee import fn +import logging + +from peewee import fn, PeeweeException from cachetools import lru_cache -from data.model import DataModelException +from datetime import datetime, timedelta + +from data.model import DataModelException, config from data.database import (Repository, User, Team, TeamMember, RepositoryPermission, TeamRole, Namespace, Visibility, ImageStorage, Image, RepositoryKind, db_for_update) +logger = logging.getLogger(__name__) + def reduce_as_tree(queries_to_reduce): """ This method will split a list of queries into halves recursively until we reach individual queries, at which point it will start unioning the queries, or the already unioned subqueries. @@ -164,3 +170,36 @@ def calculate_image_aggregate_size(ancestors_str, image_size, parent_image): return None return ancestor_size + image_size + + +def update_last_accessed(token_or_user): + """ Updates the `last_accessed` field on the given token or user. If the existing field's value + is within the configured threshold, the update is skipped. """ + threshold = timedelta(seconds=config.app_config.get('LAST_ACCESSED_UPDATE_THRESHOLD_S', 120)) + if (token_or_user.last_accessed is not None and + datetime.utcnow() - token_or_user.last_accessed < threshold): + # Skip updating, as we don't want to put undue pressure on the database. + return + + model_class = token_or_user.__class__ + last_accessed = datetime.utcnow() + + try: + (model_class + .update(last_accessed=last_accessed) + .where(model_class.id == token_or_user.id) + .execute()) + token_or_user.last_accessed = last_accessed + except PeeweeException as ex: + # If there is any form of DB exception, only fail if strict logging is enabled. + strict_logging_disabled = config.app_config.get('ALLOW_PULLS_WITHOUT_STRICT_LOGGING') + if strict_logging_disabled: + data = { + 'exception': ex, + 'token_or_user': token_or_user.id, + 'class': str(model_class), + } + + logger.exception('update last_accessed for token/user failed', extra=data) + else: + raise diff --git a/data/model/appspecifictoken.py b/data/model/appspecifictoken.py index 4a1d1c686..10130c785 100644 --- a/data/model/appspecifictoken.py +++ b/data/model/appspecifictoken.py @@ -3,10 +3,10 @@ import logging from datetime import datetime from cachetools import lru_cache -from peewee import PeeweeException from data.database import AppSpecificAuthToken, User, db_transaction from data.model import config +from data.model._basequery import update_last_accessed from util.timedeltastring import convert_to_timedelta logger = logging.getLogger(__name__) @@ -104,23 +104,7 @@ def access_valid_token(token_code): ((AppSpecificAuthToken.expiration > datetime.now()) | (AppSpecificAuthToken.expiration >> None))) .get()) + update_last_accessed(token) + return token except AppSpecificAuthToken.DoesNotExist: return None - - token.last_accessed = datetime.now() - - try: - token.save() - except PeeweeException as ex: - strict_logging_disabled = config.app_config.get('ALLOW_PULLS_WITHOUT_STRICT_LOGGING') - if strict_logging_disabled: - data = { - 'exception': ex, - 'token': token.id, - } - - logger.exception('update last_accessed for token failed', extra=data) - else: - raise - - return token diff --git a/data/model/user.py b/data/model/user.py index 654c55e64..28942898b 100644 --- a/data/model/user.py +++ b/data/model/user.py @@ -173,7 +173,7 @@ def change_username(user_id, new_username): user = db_for_update(User.select().where(User.id == user_id)).get() # Rename the robots - for robot in db_for_update(_list_entity_robots(user.username)): + for robot in db_for_update(_list_entity_robots(user.username, include_metadata=False)): _, robot_shortname = parse_robot_username(robot.username) new_robot_name = format_robot_username(new_username, robot_shortname) robot.username = new_robot_name @@ -264,15 +264,10 @@ def create_robot(robot_shortname, parent, description='', unstructured_metadata= def get_or_create_robot_metadata(robot): - try: - return RobotAccountMetadata.get(robot_account=robot) - except RobotAccountMetadata.DoesNotExist: - try: - return RobotAccountMetadata.create(robot_account=robot, description='', - unstructured_json='{}') - except IntegrityError: - return RobotAccountMetadata.get(robot_account=robot) - + defaults = dict(description='', unstructured_json='{}') + metadata, _ = RobotAccountMetadata.get_or_create(robot_account=robot, defaults=defaults) + return metadata + def update_robot_metadata(robot, description='', unstructured_json=None): """ Updates the description and user-specified unstructured metadata associated @@ -281,13 +276,7 @@ def update_robot_metadata(robot, description='', unstructured_json=None): metadata.description = description metadata.unstructured_json = unstructured_json or metadata.unstructured_json or {} metadata.save() - - -def get_robot(robot_shortname, parent): - robot_username = format_robot_username(parent.username, robot_shortname) - robot = lookup_robot(robot_username) - return robot, robot.email - + def get_robot_and_metadata(robot_shortname, parent): robot_username = format_robot_username(parent.username, robot_shortname) @@ -360,6 +349,9 @@ def verify_robot(robot_username, password): if not owner.enabled: raise InvalidRobotException('This user has been disabled. Please contact your administrator.') + # Mark that the robot was accessed. + _basequery.update_last_accessed(robot) + return robot def regenerate_robot_token(robot_shortname, parent): @@ -394,22 +386,27 @@ def list_namespace_robots(namespace): return _list_entity_robots(namespace) -def _list_entity_robots(entity_name): +def _list_entity_robots(entity_name, include_metadata=True): """ Return the list of robots for the specified entity. This MUST return a query, not a materialized list so that callers can use db_for_update. - """ - return (User - .select(User, FederatedLogin, RobotAccountMetadata) - .join(FederatedLogin) - .switch(User) - .join(RobotAccountMetadata, JOIN_LEFT_OUTER) - .where(User.robot == True, User.username ** (entity_name + '+%'))) + """ + query = (User + .select(User, FederatedLogin) + .join(FederatedLogin) + .where(User.robot == True, User.username ** (entity_name + '+%'))) + + if include_metadata: + query = (query.switch(User) + .join(RobotAccountMetadata, JOIN_LEFT_OUTER) + .select(User, FederatedLogin, RobotAccountMetadata)) + + return query def list_entity_robot_permission_teams(entity_name, include_permissions=False): query = (_list_entity_robots(entity_name)) - fields = [User.username, User.creation_date, FederatedLogin.service_ident, + fields = [User.username, User.creation_date, User.last_accessed, FederatedLogin.service_ident, RobotAccountMetadata.description, RobotAccountMetadata.unstructured_json] if include_permissions: query = (query @@ -484,6 +481,10 @@ def verify_federated_login(service_id, service_ident): .switch(FederatedLogin).join(User) .where(FederatedLogin.service_ident == service_ident, LoginService.name == service_id) .get()) + + # Mark that the user was accessed. + _basequery.update_last_accessed(found.user) + return found.user except FederatedLogin.DoesNotExist: return None @@ -749,6 +750,9 @@ def verify_user(username_or_email, password): .where(User.id == fetched.id) .execute()) + # Mark that the user was accessed. + _basequery.update_last_accessed(fetched) + # Return the valid user. return fetched @@ -990,10 +994,10 @@ def get_pull_credentials(robotname): return None return { - 'username': robot.username, - 'password': login_info.service_ident, - 'registry': '%s://%s/v1/' % (config.app_config['PREFERRED_URL_SCHEME'], - config.app_config['SERVER_HOSTNAME']), + 'username': robot.username, + 'password': login_info.service_ident, + 'registry': '%s://%s/v1/' % (config.app_config['PREFERRED_URL_SCHEME'], + config.app_config['SERVER_HOSTNAME']), } def get_region_locations(user): diff --git a/endpoints/api/robot.py b/endpoints/api/robot.py index 59b6779bb..e815498ce 100644 --- a/endpoints/api/robot.py +++ b/endpoints/api/robot.py @@ -109,7 +109,6 @@ class OrgRobotList(ApiResource): """ List the organization's robots. """ permission = OrganizationMemberPermission(orgname) if permission.can(): - include_metadata = AdministerOrganizationPermission(orgname).can() return robots_list(orgname, include_permissions=parsed_args.get('permissions', False)) raise Unauthorized() diff --git a/endpoints/api/robot_models_interface.py b/endpoints/api/robot_models_interface.py index 5783d6d5c..707fc976f 100644 --- a/endpoints/api/robot_models_interface.py +++ b/endpoints/api/robot_models_interface.py @@ -39,6 +39,7 @@ class RobotWithPermissions( 'name', 'password', 'created', + 'last_accessed', 'teams', 'repository_names', 'description', @@ -48,6 +49,7 @@ class RobotWithPermissions( :type name: string :type password: string :type created: datetime|None + :type last_accessed: datetime|None :type teams: [Team] :type repository_names: [string] :type description: string @@ -58,6 +60,7 @@ class RobotWithPermissions( 'name': self.name, 'token': self.password, 'created': format_date(self.created) if self.created is not None else None, + 'last_accessed': format_date(self.last_accessed) if self.last_accessed is not None else None, 'teams': [team.to_dict() for team in self.teams], 'repositories': self.repository_names, 'description': self.description, @@ -69,6 +72,7 @@ class Robot( 'name', 'password', 'created', + 'last_accessed', 'description', 'unstructured_metadata', ])): @@ -77,6 +81,7 @@ class Robot( :type name: string :type password: string :type created: datetime|None + :type last_accessed: datetime|None :type description: string :type unstructured_metadata: dict """ @@ -86,6 +91,7 @@ class Robot( 'name': self.name, 'token': self.password, 'created': format_date(self.created) if self.created is not None else None, + 'last_accessed': format_date(self.last_accessed) if self.last_accessed is not None else None, 'description': self.description, } diff --git a/endpoints/api/robot_models_pre_oci.py b/endpoints/api/robot_models_pre_oci.py index 46ff1f4f3..492455e81 100644 --- a/endpoints/api/robot_models_pre_oci.py +++ b/endpoints/api/robot_models_pre_oci.py @@ -24,6 +24,7 @@ class RobotPreOCIModel(RobotInterface): 'name': robot_name, 'token': robot_tuple.get(FederatedLogin.service_ident), 'created': robot_tuple.get(User.creation_date), + 'last_accessed': robot_tuple.get(User.last_accessed), 'description': robot_tuple.get(RobotAccountMetadata.description), 'unstructured_metadata': robot_tuple.get(RobotAccountMetadata.unstructured_json), } @@ -35,7 +36,8 @@ class RobotPreOCIModel(RobotInterface): }) robots[robot_name] = Robot(robot_dict['name'], robot_dict['token'], robot_dict['created'], - robot_dict['description'], robot_dict['unstructured_metadata']) + robot_dict['last_accessed'], robot_dict['description'], + robot_dict['unstructured_metadata']) if include_permissions: team_name = robot_tuple.get(TeamTable.name) repository_name = robot_tuple.get(Repository.name) @@ -54,7 +56,9 @@ class RobotPreOCIModel(RobotInterface): if repository_name not in robot_dict['repositories']: robot_dict['repositories'].append(repository_name) robots[robot_name] = RobotWithPermissions(robot_dict['name'], robot_dict['token'], - robot_dict['created'], robot_dict['teams'], + robot_dict['created'], + robot_dict['last_accessed'], + robot_dict['teams'], robot_dict['repositories'], robot_dict['description']) @@ -62,14 +66,14 @@ class RobotPreOCIModel(RobotInterface): def regenerate_user_robot_token(self, robot_shortname, owning_user): robot, password, metadata = model.user.regenerate_robot_token(robot_shortname, owning_user) - return Robot(robot.username, password, robot.creation_date, metadata.description, - metadata.unstructured_json) + return Robot(robot.username, password, robot.creation_date, robot.last_accessed, + metadata.description, metadata.unstructured_json) def regenerate_org_robot_token(self, robot_shortname, orgname): parent = model.organization.get_organization(orgname) robot, password, metadata = model.user.regenerate_robot_token(robot_shortname, parent) - return Robot(robot.username, password, robot.creation_date, metadata.description, - metadata.unstructured_json) + return Robot(robot.username, password, robot.creation_date, robot.last_accessed, + metadata.description, metadata.unstructured_json) def delete_robot(self, robot_username): model.user.delete_robot(robot_username) @@ -77,26 +81,26 @@ class RobotPreOCIModel(RobotInterface): def create_user_robot(self, robot_shortname, owning_user, description, unstructured_metadata): robot, password = model.user.create_robot(robot_shortname, owning_user, description or '', unstructured_metadata) - return Robot(robot.username, password, robot.creation_date, description or '', - unstructured_metadata) + return Robot(robot.username, password, robot.creation_date, robot.last_accessed, + description or '', unstructured_metadata) def create_org_robot(self, robot_shortname, orgname, description, unstructured_metadata): parent = model.organization.get_organization(orgname) robot, password = model.user.create_robot(robot_shortname, parent, description or '', unstructured_metadata) - return Robot(robot.username, password, robot.creation_date, description or '', - unstructured_metadata) + return Robot(robot.username, password, robot.creation_date, robot.last_accessed, + description or '', unstructured_metadata) def get_org_robot(self, robot_shortname, orgname): parent = model.organization.get_organization(orgname) robot, password, metadata = model.user.get_robot_and_metadata(robot_shortname, parent) - return Robot(robot.username, password, robot.creation_date, metadata.description, - metadata.unstructured_json) + return Robot(robot.username, password, robot.creation_date, robot.last_accessed, + metadata.description, metadata.unstructured_json) def get_user_robot(self, robot_shortname, owning_user): robot, password, metadata = model.user.get_robot_and_metadata(robot_shortname, owning_user) - return Robot(robot.username, password, robot.creation_date, metadata.description, - metadata.unstructured_json) + return Robot(robot.username, password, robot.creation_date, robot.last_accessed, + metadata.description, metadata.unstructured_json) pre_oci_model = RobotPreOCIModel() diff --git a/static/directives/robots-manager.html b/static/directives/robots-manager.html index 9acbfa5aa..54053d1b2 100644 --- a/static/directives/robots-manager.html +++ b/static/directives/robots-manager.html @@ -44,6 +44,9 @@ Created + + Last Accessed + @@ -89,6 +92,9 @@ + + + diff --git a/static/js/directives/ui/robots-manager.js b/static/js/directives/ui/robots-manager.js index eca191984..bf42eb2d2 100644 --- a/static/js/directives/ui/robots-manager.js +++ b/static/js/directives/ui/robots-manager.js @@ -41,6 +41,7 @@ angular.module('quay').directive('robotsManager', function () { }).join(','); robot['created_datetime'] = robot.created ? TableService.getReversedTimestamp(robot.created) : null; + robot['last_accessed_datetime'] = robot.last_accessed ? TableService.getReversedTimestamp(robot.last_accessed) : null; }); $scope.orderedRobots = TableService.buildOrderedItems(robots, $scope.options, diff --git a/test/data/test.db b/test/data/test.db index 46d05acbb..201814669 100644 Binary files a/test/data/test.db and b/test/data/test.db differ diff --git a/util/config/schema.py b/util/config/schema.py index 4cc4a825d..f68a78e73 100644 --- a/util/config/schema.py +++ b/util/config/schema.py @@ -66,6 +66,7 @@ INTERNAL_ONLY_PROPERTIES = { 'LOCAL_OAUTH_HANDLER', 'USE_CDN', 'ANALYTICS_TYPE', + 'LAST_ACCESSED_UPDATE_THRESHOLD_S', 'EXCEPTION_LOG_TYPE', 'SENTRY_DSN',