simplq / simplq-backend Goto Github PK

We currently have some authentication, but that is not perfect. We are using AWS Cognito, but integrating Google Sign would be easier and better for us.

Current System:Currently, a temporary userId and tempKey is generated and stored in the the local storage when someone visits simplq.me for the first time. It's used to authenticate with Cognito across sessions. This serves as an anonymous sign in. (This implementation currently has a bug: we are currently sending the identity token instead of access token, see discussion here #13)

Requirement: Users should be able to use SimplQ without needing to sign in. But at any given point, they can voluntarily sign up with us. Signing up will make sure that they can manage the queue using multiple devices, that the queue is not lost when the user switches to a new browser or clear local storage etc.

Proposal: We should use Cognito for maintaining anonymous users and then directly use Google Auth once they decide to sign up. Implementing Google sign in looks very easy: https://developers.google.com/identity/sign-in/web/sign-in

What will need to make sure that when the user signs up, all his current queues are carried over to the new identity. One possible way is to create an owner table where we store ownership information of each queue. (A one to many mapping from the unique id of the user to queueIds for which the user is an owner of). Currently we store ownership information along with the queue object.

Before the user chooses to sign in via google, this mapping will have the temp user id which we created. Once the user signs up, we will have to update this table with whatever unique identifier that google provides us.

For future: This design will allow us to have multiple owners for the same queue.

As the queue name should be directly inputable on the URL, it should not contain any spaces or special characters, and there should be UI and backend validations for the same.

To support SimplQ/simplQ-frontend#425 we need to add an extra property to the queue: MAX_QUEUE_CAPACITY.

1. When a queue is created, it will be set to -1, and -1 indicates infinite/no limit.
2. We will need to at a PATCH request to the queue resource to update the capacity limit
3. When we add a new token, we need add to the queue only if the limit has not reached.
4. /queue/status should return the remaining slot count if the field is not set to -1

  public Owner getOwnerOrElseCreate() {
    var ownerId = loggedInUserInfo.getUserId();
    return ownerRepository.findById(ownerId).orElse(ownerRepository.save(new Owner(ownerId)));
  }

According to the docs for Optional.orElse(), the statement inside orElse() (i.e ownerRepository.save(new Owner(ownerID))) is evaluated even when findById(ownerId) finds the Owner object and returns it. The reason it doesn't create problems right now is because CRUDRepository.save() merges an Object with another in the DB if the id passed is already present in the DB. This behaviour should be changed and we should resort to using orElseGet() instead as follows:

@Transactional
  public Owner getOwnerOrElseCreate() {
    var ownerId = loggedInUserInfo.getUserId();
    return ownerRepository.findById(ownerId).orElseGet(() -> ownerRepository.save(new Owner(ownerId)));
  }

This will ensure ownerRepository.save(new Owner(ownerID)) is executed only when findById finds no matching object.

Parallel to this API, define a GET request that responds with the same data, but in CSV form.

Login from http://localhost:3000/ works.
When login is attempted from https;//simplq.me it fails with message:

Our backend is very straightforward CRUD on two resources, queues and tokens. Both of them have two status fields, QueueStatus and TokenStatus.

These enums are a bit overloaded, a better approach would be remove them and have booleans, (isPaused, isRemoved) for queues, and (isRemoved, isNotified) for tokens.

ba3e6d3

URL_PREFIX is hardcoded inside Token.java and should ideally be changed to an environment file from which the value should be obtained. Hardcoding causes problems to users who might host their own instance of SimplQ on their servers.

I see the name of the queue changes and becomes capitalised in the pop-up of the qr-code. I think there might be miss-understanding arising with the name of the queue, when there are alike names like 'MyQ1' and 'Myq1'. Even, when the queue remains empty this thing happens. This might lead to confusion to the users as to which queue is the one they are wanting. I have attached the screenshots of the case below.

The above two queues are different but the same name is getting displayed. We should refrain from using duplicate queue-names irrespective of the case of the letters, as mentioned by @maaverik . So, it will be better if check if there arises duplicate queue-name irrespective of the case than just only checking duplicate queue names with case-sensitive. Like in the above example while going to make a queue of name 'Myq1' should not be allowed as there exists a queue name 'MyQ1'

Hope you understand. Please revert back for queries

We should have three profiles, dev, prod and default/local.

1. We should load a MockSmsService bean unless it's prod profile. https://github.com/SimplQ/simplQ-backend/blob/master/complete/src/main/java/com/example/restservice/service/smsService/TexLocalSmsService.java

Then we can avoid commenting out line 31 in the above file.

2. Similarly, a SecretsManager should not be loaded on default/local profile.

https://github.com/SimplQ/simplQ-backend/blob/master/complete/src/main/java/com/example/restservice/service/SecretsManager.java

These were temporary solutions that then we can then remove:
https://github.com/SimplQ/simplQ-backend/blob/master/complete/src/main/java/com/example/restservice/service/SecretsManager.java#L27
https://github.com/SimplQ/simplQ-backend/blob/master/complete/src/main/java/com/example/restservice/service/SecretsManager.java#L61

I saw a case where I had a admin page link of a queue I used before and while opening it, I'm able to see the queue status and a member. When I tried to add a member or join this queue, I see that the request fails because apparently the queue has been deleted.

{"timestamp":"2021-02-20T19:17:14.694+0000","status":422,"error":"Unprocessable Entity","message":"The queue has been deleted","reasonCode":"QUEUE_DELETED"}

Another thing is if I try to create a queue with the same name, I see an error response saying that the queue name already exists. The queue name I see this for is 'dfgdgdfg' with queue id '8775a718-c26d-42af-a757-1e03a21014ae'.

Ideally if the queue is deleted, I would expect the admin page to not fetch data and have no restriction on creating another queue with the same name. On the flip side, if it's not deleted, I should be able to join and add members. This is on the main Simplq.me site.

The default profile being used for the application runtime is local. The README also does not mention setting the profile at runtime to anything else. However, local has sms.enabled=true but does not mention how a developer can set up their credentials for the same. This leads to an error while running saying FCM Credentials are not found. The Dockerfile specifies the usage of the dev profile for running the application. I think the README should be updated with the same or mention how a user can set up the credentials.

In a step closer to moving towards public/discoverable queues and also to make the shared URL more human readable, we would need to put a uniqueness constraint on the queue name.

Handling of whitespaces, capitalization and stripping of non url special characters might be required, so names "Joy Mart" and "Joy Mart" and "Joy mart" would all be duplicates.

Whenever we update the schema, we need to migrate. How do we tackle this? currently, it fails on AWS.

Inputs
ssss
qqqq

response:-
error: "Internal Server Error"
message: "could not execute statement; SQL [n/a]; constraint [uk_4emkh34msxtqa7blxlcimoqy9]; nested exception is org.hibernate.exception.ConstraintViolationException: could not execute statement"
path: "/v1/queue"
status: 500
timestamp: "2020-09-04T22:29:35.600+0000"

Our artefact id, group id, package names, module name etc are from a sample project.

  <groupId>com.example</groupId>
  <artifactId>rest-service</artifactId>

We should refactor to me.simplq and also rename some folders

Currently SMS is enabled in prod after #68. We would need to control it via application.properties

To reproduce the issue, create a queue on the site here, add a person by any name, and then add again with a different number but same name. It will result in an error.

Looks like an unwanted unique constraint is somehow being created by hibernate or liquibase.

Error returned is:

could not execute statement; SQL [n/a]; constraint [token_name_key]; nested exception is org.hibernate.exception.ConstraintViolationException: could not execute statement

Currently backend returns only the active tokens on calling GET /queue/<queue-id>.

To support queue history, we would need to do two things:

1. Record the deletion timestamp for each token
2. Add a list of removedTokens to the QueueDetailsResponse which will have the deletion time.

A draft proposal:

We have enabled Sonarqube, it has reported some bugs and code smells. Most of these are trivial.

Report: https://sonarcloud.io/dashboard?id=SimplQ_simplQ-backend

Currently when a person joins a queue, he is asked a fixed set of input data, (name and contact number). We want to give the queue owner the ability to define a set of custom set of questions. We need to let the admin set it in a way inspired from the Google Forms model:

We need to design this first.

Can we add a parent pom.xml to the root of the repository so that importing the root folder into Intellij would also work?

Otherwise we should add instruction to open simplq/ folder into Intellij.

cc @dhirenmathur

We have different tests on queue and token entity objects done all over the place.

This should be extracted into other methods and use function composition. It is very hard to follow.

  Predicate<Queue> isNotPaused() {
    return queue -> {
      if (queue.getStatus() == QueueStatus.PAUSED) {
        throw SQInvalidRequestException.queuePausedException();
      }

      return true;
    };
  }

  Predicate<Queue> isNotDeleted() {
    return queue -> {
      if (queue.getStatus() == QueueStatus.DELETED) {
        throw SQInvalidRequestException.queueDeletedException();
      }

      return true;
    };
  }

  Predicate<Queue> isNotFull() {
    return queue -> {
      if (queue.isFull()) {
        throw SQInvalidRequestException.queueDeletedException();
      }

      return true;
    };
  }

  void validateQueue(Queue queue) {
    isNotPaused()
        .and(isNotDeleted())
        .and(isNotFull())
        .test(queue);
  }

and then just call validateQueue(queue)

Originally posted by @chalx in #130 (comment)

A queue creator can optionally choose to protect his queue by using a 4 digit pin. Then he can keep refreshing the pin whenever he wishes to.

Can we use Lombok to reduce some boiler plate code?

We should write one such testcase, which creates a queue, and then create a token in the queue. The in-memeory DB should work. Even asserting 200 OK is enough for now.

While travis.yml file is hidden from the github UI it isn't good practise to have a hardcoded token, we should store the token in Github secrets for the repository and use an environment variable to access it in the script.

This should have no impact on local dev environment setup as travis isn't used anywhere locally.

Placeholder issue, will update soon with more details.

We need to do DB migrations from CI automatically

Aim: Avoid downtime by proactive monitoring.

Cloudwatch in free tier allows 10 custom alarms and metrics. It can be used for this.

1. If cpu/memory/(disk utilisation) goes above a threshold for the , alert send to dev team. If for some reason the server also goes down, or a EC2 health check failed, then also some notification should be set up.

2. If there is unusually high rate of HTTP 500s or 400s, alert send to dev team
   For two we need to start using API Gateway so that these metrics star coming to cloudwatch. So blocked on #64

3. Logs and metrics should be stored.

Currently the logs are in syslog. Can we sent it to cloudwatch?

Also if the two services restart more than 3 times in half an hour, can we get an alert?

As a first step to enable self hosting of SimplQ, we need to write a build script and a Dockerfile.

For now created an empty repository simplQ-docker and documented the requirements in the repo's Readme here.

There would be a join queue link, and when the user types a queue name, list of all queues that meet the search text criteria would be returned.

For a simple model, a prefix search can be done.

Have to think more, but thinking of writing a health check lambda scheduled to hit the backend server periodically, and send an email alert if the health check fails.

Hi,

Not sure if this is a feature or an issue. I am not clear as to why once a queue having a certain queue name is deleted, a new queue with the same name can't be created again.

This creates unexpected behaviour.

A strong point which brought back token number to the table was the scenario where two people with the same name are present in the queue. So, we decided that it would be required to also generate a running (1, 2, 3, 4..) tokenNumber for each user.

This will not replace anything in the current system. This is different from tokenId which is the UUID/PRIMARY_KEY generated for the user joining the queue.

Task: Add an extra tokenNumber field to the DB, and return it with tokens (GET /queue/{queueId} and GET /token/{tokenId})

There would be a check box or a toggle button on the UI, that the creator would set, according to which the queue would be marked as public or private.

Public Queues will later be the ones that are searchable.

In the current system, anyone with queueID can become the admin. For example, anyone with the queueID can send requests to the backend directly, and say, remove people from the queue.

The right real issue here is that we don't authenticate users. The proper solution to this is to implement authentication

When a new token is created, we want to send an SMS to the mobile number which was given. This SMS would have a link to the token status page, this will allow the person to know the live status while he is waiting, mainly the number waiting in front of him:

Hi <name>, 
You have been added to  <queue-name>. 
Live status: <token-status-link>.
Token number: <token-number>.

AWS SNS

AWS SNS lets you send SMS worldwide, and and their pricing is as follows (converted to Rupees):

If we adhere to some India specific regulations which might require us to be a registered company, and also might require us to pay an additional registration fee (₹5900) then we can send SMS in India for Rs. 0.20.

Direct Integration With SMS Service Providers

We can integrate with an SMS service provider, whose API we can call to trigger the SMS. Integration is very easy, you just have to set up an account, pay as per their plan. A random example service I found for India is TextLocal

@akashkbaburajan was exploring this, and I think we will need to be registered as a company to send transactional SMSes.

Companion App to Send SMS

We could also let the queue owner install and set up a SimplQ companion app on his mobile phone to send SMSes in the background. This was discussed here.

Other thoughts

1. Except in case of the companion app, SMS is a paid resource that we cannot leave open on a public site without protecting it, otherwise someone might use it to spam others or will simply consume our quota. We can get around this by allowing only logged in users to enable SMS, and if they want to send more that a certain amount of SMS, they would have to contact us offline and we can increase their limit.
2. We would integrate multiple providers and use the best one per each region. Also we should enable SMS as a feature only for certain regions (for example, enabling SMS for Ukraine is very costly). So we might end up using AWS for UK and US, while another service like TextLocal for India etc.
3. If the companion app is feasible, we can offload this responsibility to the user, and also keep the platform free for ever. We could always do all.

But SMS does seem like a good feature to have, from a product perspective.

Current backend deployment is fully manual. We need to automate.

Also there is only one instance now. Dev server like we have for UI is needed.

Now that auth is here, and we also know the user name of the person who is joining the queue, we can restrict that a person can join the queue only once.

He can join the queue again once he had left the queue. In other words, the same guy can't join the queue and be in the queue twice.

The UI, on going to to join queue page, should handle this. @akashkbaburajan

Our backend service goes down randomly. This happened today while I have shell access today. Looks like a memory leak.

Disk

df -lh
Filesystem      Size  Used Avail Use% Mounted on
udev            476M     0  476M   0% /dev
tmpfs            98M  820K   98M   1% /run
/dev/xvda1      7.7G  7.1G  673M  92% /
tmpfs           490M     0  490M   0% /dev/shm
tmpfs           5.0M     0  5.0M   0% /run/lock
tmpfs           490M     0  490M   0% /sys/fs/cgroup
/dev/loop1       56M   56M     0 100% /snap/core18/1932
/dev/loop3       77M   77M     0 100% /snap/bombsquad/3
/dev/loop4       62M   62M     0 100% /snap/core20/875
/dev/loop5       29M   29M     0 100% /snap/amazon-ssm-agent/2333
/dev/loop6       90M   90M     0 100% /snap/bombsquad/4
/dev/loop8       98M   98M     0 100% /snap/core/10444
/dev/loop9       33M   33M     0 100% /snap/amazon-ssm-agent/2996
/dev/loop10      62M   62M     0 100% /snap/core20/904
/dev/loop0       98M   98M     0 100% /snap/core/10577
/dev/loop11      56M   56M     0 100% /snap/core18/1944
tmpfs            98M     0   98M   0% /run/user/1000

Memory:

              total        used        free      shared  buff/cache   available
Mem:           978M        868M         61M        868K         48M         15M
Swap:            0B          0B          0B

Top Output:

top - 10:31:20 up 22 days,  2:07,  1 user,  load average: 24.29, 16.00, 7.62
Tasks: 124 total,   2 running,  81 sleeping,   0 stopped,   0 zombie
%Cpu(s):  0.3 us, 18.0 sy,  0.0 ni,  0.0 id, 80.6 wa,  0.0 hi,  0.2 si,  0.9 st
KiB Mem :  1002124 total,    71044 free,   891008 used,    40072 buff/cache
KiB Swap:        0 total,        0 free,        0 used.    19312 avail Mem 

  PID USER      PR  NI    VIRT    RES    SHR S %CPU %MEM     TIME+ COMMAND                                                                                                  
28294 root      20   0  0.619g 0.014g 0.000g S  6.0  1.4   1:28.08 snapd                                                                                                    
   82 root      20   0  0.000g 0.000g 0.000g S  5.4  0.0   0:45.90 kswapd0                                                                                                  
 1227 root      20   0  0.702g 0.013g 0.000g S  2.6  1.4   1:03.32 ssm-agent-worke                                                                                          
  790 root      20   0  0.686g 0.008g 0.000g S  2.0  0.9   1:06.09 amazon-ssm-agen                                                                                          
11060 ubuntu    20   0  2.298g 0.197g 0.000g S  1.3 20.6   0:38.65 java                                                                                                     
   94 root       0 -20  0.000g 0.000g 0.000g I  0.8  0.0   0:08.22 kworker/0:1H-kb                                                                                          
  832 ubuntu    20   0  2.306g 0.249g 0.000g S  0.6 26.0  27:52.29 java                                                                                                     
   10 root      20   0  0.000g 0.000g 0.000g S  0.4  0.0   0:08.22 ksoftirqd/0                                                                                              
    1 root      20   0  0.215g 0.003g 0.000g D  0.4  0.3   0:24.02 systemd                                                                                                  
  835 root      20   0  0.632g 0.016g 0.000g S  0.4  1.7  14:11.97 containerd                                                                                               
11841 root      20   0  0.024g 0.003g 0.000g R  0.4  0.3   0:02.33 lsb_release                                                                                              
11853 ubuntu    20   0  0.042g 0.001g 0.001g R  0.3  0.1   0:00.47 top                                                                                                      
  393 root      19  -1  0.117g 0.011g 0.000g D  0.2  1.2   0:25.07 systemd-journal                                                                                          
11744 root      39  19  0.192g 0.072g 0.000g D  0.2  7.5   0:21.22 apt-check                                                                                                  
28175 root       0 -20  0.000g 0.000g 0.000g D  0.2  0.0   0:01.83 loop0                                                                                                    
  462 root       0 -20  0.000g 0.000g 0.000g D  0.2  0.0   0:02.05 loop9                                                                                                    
 1099 root      20   0  0.521g 0.126g 0.000g D  0.2 13.2   3:01.21 ruby                                                                                                     
11850 root      20   0  0.063g 0.001g 0.000g D  0.2  0.1   0:00.50 sshd                                                                                                     
11858 root      20   0  0.056g 0.000g 0.000g S  0.1  0.0   0:00.14 cron                                                                                                     
   11 root      20   0  0.000g 0.000g 0.000g I  0.1  0.0   0:13.86 rcu_sched                                                                                                
  789 root      20   0  0.275g 0.001g 0.000g S  0.1  0.1   0:30.52 accounts-daemon                                                                                          
11070 root      20   0  0.000g 0.000g 0.000g I  0.1  0.0   0:00.56 kworker/0:1-eve                                                                                          
11246 ubuntu    20   0  0.103g 0.001g 0.000g S  0.1  0.1   0:00.11 sshd                                                                                                     
  774 root      20   0  0.030g 0.000g 0.000g S  0.0  0.0   0:02.77 cron                                                                                                     
  830 syslog    20   0  0.255g 0.002g 0.000g S  0.0  0.2   0:04.88 rsyslogd                                                                                                 
 1086 root      20   0  0.112g 0.013g 0.000g S  0.0  1.4   1:17.50 ruby 

Both the java processes (prod and dev backend services) are taking 2GB+ each, which should not be happening.

Originally posted by @daltonfury42 in #61 (comment)

We store the creation timestamp of token and queue in DB. We need to make sure it's UTC time