OLH-1053 update reported flag on activity

[saralk]

Proposed changes

When a user reports an activity as suspicious, we should record that in the activity log, so that we can ensure we don't report things twice, and provide feedback to the user.

What changed

• Create a new lambda, and all the infrastructure that goes along with that (log groups, policies etc...), in the CloudFormation template
• Add a secondary index to the activity_log table, so that we can query the table based on the event_id
• When the new lambda is triggered, we retrieve the event from the event log (based on the event_id), and update it, setting reported_suspicious to true

Why did it change

When we build functionality for users to report activity as suspicious, we want to a) give feedback to the user, and b) ensure that we don't make multiple tickets in zendesk.

By setting this flag to true, we can show that to the user, so that they know we have received the report. This will work even if there is an issue with Zendesk, and we haven't got a ticket number yet.

Related links

#181

Checklists

Environment variables or secrets

• Added parameters to Cloudformation template
• Added new environment variable

Testing

I manually tested this by...

Deploying the changes to the dev environment

Getting an event_id from the activity_log table (through the AWS console)

Publishing a message on the SuspiciousActivity topic with the following body (replacing event_id with the one I got in the previous step)

{
  "event_id": "b62c6ec9-30e6-4f7d-9769-02e25b81baf6",
  "event_name": "event_name",
  "timestamp": 1609462861,
  "timestamp_formatted": "timestamp_formatted",
  "client_id": "client_id",
  "user": {
    "user_id": "user_id",
    "session_id": "session_id",
    "govuk_signin_journey_id": "govuk_signin_journey_id"
  },
  "reported_suspicious": true
}

Refreshing the DyanamoDB view, and checking that the reported_suspicious property has been set to true for the event I chose.

[alex9smith]

This looks really good - nice one!

I'd forgotten this table still had a composite primary key of user_id and timestamp which I think has made your work more complicated than it should have been. Sorry about that. We should have already updated the table first to use the new primary key of user_id and event_id we defined in ADR 0010. You've done the right thing here in working with the primary key as it it but it does need to change for this to be robust as we should expect to see multiple events for a user with the same timestamp at some point since timestamp is only to the nearest second.

I think what we should do is update the indices now in your PR. Then you could get rid of the getItemByEventId function and update markEventAsReported to use the user_id and event_id keys. We can rely on DynamoDB to enforce uniqueness for the primary key. We'll also be able to get rid of the global secondary indices which will save us a bit of money.

The first part of the table definition would then look like this:

 UserActivityLogStore:
    Type: AWS::DynamoDB::Table
    DeletionPolicy: Delete
    UpdateReplacePolicy: Delete
    Properties:
      AttributeDefinitions:
        - AttributeName: user_id
          AttributeType: S
        - AttributeName: event_id
          AttributeType: S
      BillingMode: PAY_PER_REQUEST
      TableName: !Ref ActivityLogStoreTableName
      KeySchema:
        - AttributeName: user_id
          KeyType: HASH
        - AttributeName: event_id
          KeyType: RANGE

Again, sorry for making this harder than it needed to be! Let me know if you want to chat about any of this in more detail.

[saralk]

Make sense, thanks @alex9smith.

I wonder if it's worth doing that in a separate ticket, as there are a places in other lambdas where the timestamp is used in a query (here for example).

So we'd have to make the change there, increasing the scope of this PR beyond what's required in the ticket.

[alex9smith]

Ah yeah - that's a good point. I'd forgotten this was still in the delete lambda as well so I thought it'd still be only changes to this lambda. I'll write up a ticket to change the primary key for next sprint.