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

Support for .ecmwfapirc file #11

Closed
eliocamp opened this issue Mar 18, 2019 · 14 comments
Closed

Support for .ecmwfapirc file #11

eliocamp opened this issue Mar 18, 2019 · 14 comments

Comments

@eliocamp
Copy link
Collaborator

Right now the package uses a keyring, but it's also possible to use a .ecmwfapirc file in the home folder. This method has the advantage that works with the python module too, so if someone is working with both versions there's no need to have credentials in two places.

How about adding support for both methods? A good way of doing this would be to use the information in .ecmwfapirc if no user is passed to wf_request(). This would also have the side benefit of being able to have a default user (in my case, I have only one user so the user parameter is redundant).

Something like this should work (and some modifications to wf_request())

wf_get_key <- function(user){
  if (missing("user")) {
    creds <- .read_ecmwfapirc()
    message(paste0("Using ", creds$user, " credentials from .ecmwfapirc"))
    creds$key
  } else {
    keyring::key_get(service = "ecmwfr", username = user)
  }
}

#  Function to locate and parse .ecmwfapirc
# (only tested on linux!)
.read_ecmwfapirc <- function() {
  os_type <- Sys.info()["sysname"]
  if (os_type == "Windows") {
    file_path <-  file.path(Sys.getenv("USERPROFILE"), ".ecmwfapirc")
  } else {
    file_path <- file.path(Sys.getenv("HOME"), ".ecmwfapirc")
  }
  lines <- readLines(file_path)

  parse_data <- function(name) {
    lines <- lines[grepl(name, lines)]
    lines <- strsplit(lines, ":")[[1]][2]
    lines <- gsub('"', "", lines, fixed = TRUE)
    lines <- gsub(" ", "", lines, fixed = TRUE)
    lines
  } 
  
  key <- parse_data("key")
  user <-   parse_data("email")
  list(user = user, key = key)
} 

@khufkens
Copy link
Member

khufkens commented Mar 18, 2019

I do not give users the option intentionally because in so many ways the plain text credentials are really bad practice. Basically, you should not encourage bad behaviour. With a proper keyring you should only set this up once. So, this one time copy paste is a minor inconvenience while really limiting leaking credentials (to github or elsewhere).

@khufkens
Copy link
Member

In short, I'm reluctant to include the code. Not because it ain't good code, rather than nudging people in the right direction when using credentials (certainly when the infrastructure is in place).

@eliocamp
Copy link
Collaborator Author

I can see that.
Are you open to a function that opens a browser to https://api.ecmwf.int/v1/key/ and interactively saves the credentials in the keyring? Something like this:

wf_get_credentials <- function() {
    browseURL("https://api.ecmwf.int/v1/key/")
    message("Login or register to get a key")
    user <- readline("Email: ")
    key <- readline("API key: ")
    
    keyring::key_set_with_value("ecmwfr", user, password = key)
}

(I think using this as the behaviour when wf_set_key() is called with no argument could be user friendly)

@khufkens
Copy link
Member

If this works, yes. Not sure how you parse the key from the page however. Always happy to learn.

Keep in mind that the package provides coverage for both the webapi and CDS. So it would be best to have this work for both I think.

Inconsistency in user experiences will leave people confused. My experience with teaching demos for other packages is that the moment you deviate from a certain way of dealing with things you will confuse people more than you would know. Best to stick to a consistent rule set (functions).

@khufkens
Copy link
Member

Just to be clear, if you can't get CDS to work be sure to use a message to point people in the right direction. Being verbose is rather key in these cases.

@eliocamp
Copy link
Collaborator Author

I was thinking of asking the user to paste it in the terminal an read it with readlines. Human parsing, hehe.

@khufkens
Copy link
Member

Ah in that way, yes. Good idea.

Ideally we could pass the user / password combo in R to the site login, and grab all required info. I think this should be possible. I'm however not sure how secure this would be.

Maybe do it the human parsing way first?

@eliocamp
Copy link
Collaborator Author

Is the low-hanging fruit. Scrapping the key would require new dependencies and it might not be possible (polite) depending on the robots.txt file.

@khufkens
Copy link
Member

True, this works for the portal login regardless (for further reference):

login <- "https://apps.ecmwf.int/auth/login/security-token/?back=https://api.ecmwf.int/v1/key/"
pars <- list(
  uid = "xxx",
  password = "xxxx"
)
ct <- POST(login, body = pars)
text <- content(ct, as = "text")
read_html(text) %>% html_node(".token") %>% html_text()

@khufkens
Copy link
Member

Robots is here, I'm not sure how to read this as the endpoint isn't mentioned. https://www.ecmwf.int/robots.txt

@eliocamp
Copy link
Collaborator Author

You can use the package robotstxt for that.

@eliocamp
Copy link
Collaborator Author

I was thinking about the other method. Following your idea of not encouraging bad behaviour, is it wise to encourage people to type their credentials into some random R package? Having this option would also allow for people to write scripts with their user and password in the code; maybe as a setup process to run in new computers or fresh installations.

@khufkens
Copy link
Member

If it is in the code it is visible, if it is in a hidden rc file people forget about it and the permissions they forgot to set properly (and if not upload it to places unintentionally). The hidden status of the file is security through obfuscation, which is rather easily foiled.

On major systems I worked (HPC) user's home directories were generally not encrypted. So, if permissions were not set properly you could read all files without fail. This is the kind of issue you run into without an encrypted keychain. The below grep command dumps all password matches (with passw as a key) in no time flat.

grep -ir "passw" .

Technically you need to set your permissions to 600 when sharing a system, especially.

chmod 600 .yourrc

Neither the official install documentation of ECMWF or the CDS mention this. Just showing where the holes are people will not be aware of!

In short, when it is in your R code you are aware of the visibility of it - less likely to keep it this way. When you use a plain text file, not so much (especially since people aren't even instructed to change permissions). And if your system gets compromised it will be easy game.

As to punching code into an unknown program. The source code is open and it goes through code review before it is published on CRAN. The latter is a mark of approval. It is not 100% but is more than say what you get on the python pip system which lacks code review.

I doubt you read the python code of the ecmwf python api before you installed or ran it? But, are you sure it doesn't phone home with logs of your keystrokes? You assume that this doesn't happen because you trust ECMWF.

In any case, when you query the password through the readLines() method you suggested there will be actually no trace nowhere of the password in plain text. This is actually better than current, as there might be a trace of the key in the history file for some number of commands.

@khufkens
Copy link
Member

I'm going to close this. I think the new functionality of wf_set_key() with dynamic input if not redirection to the correct pages with info should make this process ok without the need for the plain text file.

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

2 participants