Restcomm-Connect icon indicating copy to clipboard operation
Restcomm-Connect copied to clipboard

Published 20 hours ago • verified publisher icon RestComm

Patch for Multi API Auth tokens

Open arslan70 opened this issue 7 years ago • 6 comments

Please review the changes for a patch for implementing Multiple API Auth tokens

Changes Overview ===DB/DAO Changes===

  1. Changed mybatis version from 3.2.2 to 3.4.4
  2. Removed auth_token field from restcomm_accounts table
  3. Added table restcomm_accounts_auth_tokens
  4. Changed Mybatis accounts.xml file
  5. Changed Initialization DB Script

===Code Changes===

  1. Endpoints added for adding, deleting and listing auth tokens in http module
  • AddAuthToken()
  • GetAuthTokens()
  • DeleteAuthToken()

  1. Authentication logic changed for matching password. Matching is done against a list in containsAuthToken(token) in Account.java

  2. Added test cases related to auth tokens in AccountsDaoTest

arslan70 avatar Jun 07 '17 06:06 arslan70

@arslan70 Have you thought through how we'd migrate data upon install of this patch? I don't see a migration script to move the data from the current table to the new table as a part of the update.

Something to think about. This all looks really great as a first cut. I've made a few small notes, will look at it again in the next couple days.

scottbarstow avatar Jun 07 '17 21:06 scottbarstow

Hey Scott, I just removed the System.outs

I'll work on the migration script. Is it possible to have a sample data table for me to run and test the script?

arslan70 avatar Jun 08 '17 06:06 arslan70

@arslan70 Just create a test case that creates a bunch of accounts and then executes the migration, then tests the results. That would be best. We can also do full migration testing, but the first step is a test or set of tests.

scottbarstow avatar Jun 08 '17 12:06 scottbarstow

Thanks @arslan70

@maria-farooq @gvagenas can you please help with the review ?

deruelle avatar Jun 20 '17 07:06 deruelle

Hi @gvagenas , thanks for the review and suggestions. I'll pick up the work but I just wanted to let you know that there was another issue I was working on for which I did a PR. That PR handles the password for web. This PR handles token authentication and management from API's perspective. So I'm picking it up after some time and may need your help. Right now can you review this PR again ignoring the code which is written for password authentication/management?

arslan70 avatar Aug 23 '17 17:08 arslan70

@gvagenas

Also all the tests I run in the testsuite failed because HSQLDB script is not updated. You need to take care of that also.

Let's get this working after my branch is updated and github is telling me there are some conflicts to be resolved :(

arslan70 avatar Aug 23 '17 17:08 arslan70