Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ct_register_token() does not validate token #34

Closed
amannj opened this issue Aug 3, 2021 · 4 comments
Closed

ct_register_token() does not validate token #34

amannj opened this issue Aug 3, 2021 · 4 comments

Comments

@amannj
Copy link
Contributor

amannj commented Aug 3, 2021

Hi Chris,

I think comtradr::ct_register_token() fails to verify if the provided token is recognised by Comtrade's API:

> comtradr::ct_get_remaining_hourly_queries()
[1] 100
> comtradr::ct_register_token('badtoken')
> comtradr::ct_get_remaining_hourly_queries()
[1] 10000

Maybe an easy way to check if a token is valid could be via getUserInfo. Maybe something like:

mytoken <- "..."
rjson::fromJSON(
  file = paste0("https://comtrade.un.org/api/getUserInfo?token=", mytoken)
)
@wreesman
Copy link

wreesman commented Nov 9, 2021

I believe I am having this exact issue: I have credentials for a premium account and thus a token, which I obtained from https://comtrade.un.org/api/swagger/ui/index#!/Auth/Auth_Authorize, and I have verified the validity of the token via https://comtrade.un.org/ws/CheckRights.aspx (which says that I have web service access using the token to the Comtrade database), but when I pass the token in R via ct_register_token() it does not seem to be verifying, as when I then run ct_get_remaining_hourly_queries() the value has not changed from prior to passing the token (i.e. still 100, not 10,000). It appears that subsequent data pull requests are subject to guest user limits, as opposed to premium user limits that the token should provide me with.

@amannj
Copy link
Contributor Author

amannj commented Nov 15, 2021

Hi all,

@wreesman, I'm not sure if these issues are necessarily related:

  1. What seems to be happening is that as long as you provide any token, the package will tell you that you have premium access rights. I'll demonstrate this below.

  2. Another question is if the package actually uses your valid premium access credentials when you run ct_search(). I have no answer to that at the moment.

For now, I want to demonstrate issue (1):

devtools::install_github("ropensci/comtradr")
library(comtradr)

See what happens when we provide no token:

# Check behaviour: no token
ct_register_token(NULL) 
getOption("comtradr")
## $comtrade
## $comtrade$token
## NULL
## 
## $comtrade$account_type
## [1] "standard"
## 
## $comtrade$per_hour_limit
## [1] 100
## 
## $comtrade$per_second_limit
## [1] 1
## 
## 
## attr(,"class")
## [1] "comtradr_credentials"
ct_get_remaining_hourly_queries()
## [1] 100

So far, so good. Now, what if we add some random/invalid token instead?

# Check behaviour: some random/invalid token
ct_register_token("badtoken")
ct_get_remaining_hourly_queries()
## [1] 10000
getOption("comtradr")
## $comtrade
## $comtrade$token
## [1] "badtoken"
## 
## $comtrade$account_type
## [1] "premium"
## 
## $comtrade$per_hour_limit
## [1] 10000
## 
## $comtrade$per_second_limit
## [1] 1
## 
## 
## attr(,"class")
## [1] "comtradr_credentials"

Here things go wrong. This is the initial issue #34 I reported.

Potential solution

I just pushed a fix to branch token-validation. Now, ct_register_token() checks if the token you provide is recognised by the API. If not, you keep your “guest” rights; if it is, you get upgraded to “premium”.

Can you try installing from that new branch? Can you please tell me if you now observe a similar behaviour to what I show below?

Start with a fresh R session and add your authentication token to R; mine is stored in variable goodtoken. Next, install comtradr from branch token-validation:

require(remotes)
remotes::install_github("amannj/comtradr", ref = "token-validation")

See what happens when we provide no token:

# Check behaviour: no token
ct_register_token(NULL) 
getOption("comtradr")
## $comtrade
## $comtrade$token
## NULL
## 
## $comtrade$account_type
## [1] "standard"
## 
## $comtrade$per_hour_limit
## [1] 100
## 
## $comtrade$per_second_limit
## [1] 1
## 
## 
## attr(,"class")
## [1] "comtradr_credentials"
ct_get_remaining_hourly_queries()
## [1] 100

Same as before. Now with a rubbish token:

# Check behaviour: some random/invalid token
ct_register_token("badtoken")
ct_get_remaining_hourly_queries()
## [1] 100
getOption("comtradr")
## $comtrade
## $comtrade$token
## NULL
## 
## $comtrade$account_type
## [1] "standard"
## 
## $comtrade$per_hour_limit
## [1] 100
## 
## $comtrade$per_second_limit
## [1] 1
## 
## 
## attr(,"class")
## [1] "comtradr_credentials"

Good. Finally, let's try a legit one:

ct_register_token(token = goodtoken)
getOption("comtradr")
## $comtrade
## $comtrade$token
## [1] "....your legit token should show up here...."
## 
## $comtrade$account_type
## [1] "premium"
## 
## $comtrade$per_hour_limit
## [1] 10000
## 
## $comtrade$per_second_limit
## [1] 1
## 
## 
## attr(,"class")
## [1] "comtradr_credentials"
ct_get_remaining_hourly_queries()
## [1] 10000

Looks good. The counter seems to be working as well:

ex_1 <- ct_search(reporters = "China",
                  partners = c("Rep. of Korea", "USA", "Mexico"),
                  trade_direction = "exports")

ct_get_remaining_hourly_queries()
## [1] 9999

As I said before, I don't know if this also eliminates issue (2). For this, I think, we'd have to check if we still run into guest-user restrictions when sending out your queries. If we do, we should probably create a new issue.

@ChrisMuir: In my fix, I also had to change test-ct_register_token, because token = "some_token_str" now does not “upgrade” your account_type to premium, and devtools::check() would have thrown an error without the correction. I hope you don't mind.

@wreesman
Copy link

@amannj, your potential solution worked on my end; the hourly queries command is now updating correctly when I pass my token. I have no insight into issue (2), however. If I run into hitches related to that as I am making my queries, I will let you know.

@amannj amannj mentioned this issue Nov 17, 2021
@ChrisMuir
Copy link
Member

Resolved in PR-37.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants