@kiku1705 hi here it is forgot to @

Here @xihai01, here it is, thanks for reviewing my pr! @kiku1705 can't.

coolios

@7riumph just for clarification on how the sign out should work
When the user signs out, we want to clear both the access and refresh digest tokens from their respective api credentials table and then maybe replace them with a null value or something
When the same user signs in again, a new set of access and refresh digest tokens will be generated and stored in their api credentials table - and the token regeneration is already taken care of by the sign in route

Right?

@xihai01 Yes, this exactly, and would assign them to nil. Regeneration is already taken care of in the sign_in route. This sign_out route will wipe the previous token values when called.

awesome. Thanks for clarifying
I'll aim to get something out by end of this week.

@7riumph btw, what should the response schema be?
Do we want to send any sort of sign out messages back?
or maybe leave the response body empty?

@xihai01 Yes sure, sign-out error handling in the response body would be useful information for the client / iOS app, a'la sending "Sign-Out Success" or "Sign-out Error" on top the response status.

    it "creates a secure hashed api_token" do
      api_credential.api_token_digest
      api_token = api_credential.return_new_api_token![:api_token]

      expect(api_credential.api_token_digest).to eq(Digest::SHA256.hexdigest(api_token))
    end
  end

@7riumph I'm looking at your model test for the api credentials table and I am a bit confused on the expect part.
How are you able to know that both digests are the same?
Cuz in the factory for api credential, the tokens are also randomly generated.

@xihai01 Cause when hashing, equal strings, it always gives the same result (or it should at least, that's why I added the test)

e.g 

api_token = "12345"

api_token_digest_1 = Digest::SHA256.hexdigest(api_token)

api_token_digest_2 = Digest::SHA256.hexdigest(api_token)

api_token_digest_1 == api_token_digest_2

Potentially redundant, but if the test doesn't pass there's definitely a problem 😅

@7riumph
I think I am confused on how secure random hex works
like in the factory for api credential, you are also calling secure random hex before hashing it
How do you know that the string generated from the secure random hex in the factory is the same though?

FactoryBot.define do
  factory :api_credential do
    association :user
    api_token_digest { Digest::SHA256.hexdigest(SecureRandom.hex(18)) }
    refresh_token_digest { Digest::SHA256.hexdigest(SecureRandom.hex(18)) }
    token_expires_at { 1.hour.from_now }
    refresh_token_expires_at { 1.day.from_now }
  end
end

do you think it would be better to use a hard coded string? So the results are more predicatble in tests

FactoryBot.define do
  factory :api_credential do
    association :user
    api_token_digest { Digest::SHA256.hexdigest(SecureRandom.hex(18)) }
    refresh_token_digest { Digest::SHA256.hexdigest(SecureRandom.hex(18)) }
    token_expires_at { 1.hour.from_now }
    refresh_token_expires_at { 1.day.from_now }
  end
end

For our use case, it's just an 18 character string, but randomly generated for security 😎

right but is it the same 18 character string as the one generated by the model's (return_new_api_token![:api_token]) method?
because for this method, it is also calling secure random hex
So because it is randomly generated, the two strings should be different from each other
but i am confused on how you are able to expect to_eql for both of these strings in your test lol

FactoryBot.define do
  factory :api_credential do
    association :user
    api_token_digest { Digest::SHA256.hexdigest(SecureRandom.hex(18)) }
    refresh_token_digest { Digest::SHA256.hexdigest(SecureRandom.hex(18)) }
    token_expires_at { 1.hour.from_now }
    refresh_token_expires_at { 1.day.from_now }
  end
end

do you think it would be better to use a hard coded string? So the results are more predicatble in tests

Can defiantly hard code if you'd like.

right but is it the same 18 character string as the one generated by the model's (return_new_api_token![:api_token]) method? because for this method, it is also calling secure random hex So because it is randomly generated, the two strings should be different from each other but i am confused on how you are able to expect to_eql for both of these strings in your test lol

No it's not. I get what you mean, though. This is to-do with the different environments, the factories are mock data for QA on the 1st load up. So that all QA users have randomly generated tokens by default to start.

Past then, return_new_api_token! is used.

@xihai01 Speaking of qa, actually still need to get Linda or Shen to run the rake task in qa, because rn the tokens cant be fetched there without error as they are are empty or nil. So you're on the money, thanks for your attention to detail.

lol I'm still a bit confused on that test posted above passes
To me, I would think it fails because here is my understanding of the code:

api_credential.api_token_digest -> tbh I have no clue what this is supposed to do - is it setting something up? cuz I don't see it being assigned to any variable

api_token = api_credential.return_new_api_token![:api_token] -> this line is straighforward. It is calling the return_new_api! method to generate a 18 character string that is using secure random hex and returns the plain api token and stores its digest in the db

expect(api_credential.api_token_digest).to eq(Digest::SHA256.hexdigest(api_token)) -> this is the part I am confused. I get we are comparing both digests but its how the digests are generated I don't completely understand.

Digest::SHA256.hexdigest(api_token) returns a digest of the string held inside api_token variable

api_credential.api_token_digest is returning the digest of a secure random hex string from the factory

How is it that both digests are the same lol
cuz both digests were generated from hashing secure random hex functions

I would assume if I called secure random hex each time, the strings generated would be unique and then comparing their equality, I would get false:

for example, if I call secure random hex and then hash it 2 times and did a comparison between both digests:

1. some_hashing_function(secure random hex function)
2. some_hashing_function(secure random hex function)

both 1) and 2) when compared should be different digests
That's what I would assume.

wait
or are you saying api_token = api_credential.return_new_api_token![:api_token] overrides and updates the api credentials table again with the digest of api_token?

wait or are you saying api_token = api_credential.return_new_api_token![:api_token] overrides and updates the api credentials table again with the digest of api_token?

Yes, exactly.

@xihai01 All good now? Can hop on a call tomorrow as well. And potentially separate things out if there's clarity issues.

@7riumph yup I basically understand it now. I'm still confused on the first line tho - api_credential.api_token_digest
what's the purpose of calling that there?

@xihai01 This format initializes the variable within said Rspec credentials test (refresh and api token). Without this line(s), the tests failed for me, though my local Rspec is finicky.

Please check if the initialization is necessary for yourself locally, and upon making your pr 🔥

This issue has been inactive for 246 hours (10.25 days) and will be unassigned after 114 more hours (4.75 days). If you have questions, please

If you are still working on this, comment here to tell the bot to give you more time