openforis / sepal_ui

wrapper for ipyvuetify widgets to unify the display of voila dashboards in the SEPAL plateform
https://map-app.up.railway.app
MIT License
13 stars 4 forks source link

update gee authentication process #905

Closed dfguerrerom closed 5 months ago

dfguerrerom commented 9 months ago

WIP

dfguerrerom commented 8 months ago

hi @12rambau , would you mind telling me what is the format of the EARTHENGINE_TOKEN that is stored in secrets? I have tested with my credentials on my local actions and I get a different error, the format of my credentials is:

{"client_id": "", "client_secret": "", "refresh_token": ""}

In any case I think we will have to add the project_id key, do you think you can check this?

dfguerrerom commented 8 months ago

I have copied the authentication process from the proposal on https://github.com/gee-community/pytest-gee/pull/24. I also have updated the tests here, everything should work if we add $EARTHENGINE_PROJECT var to the secrets.

12rambau commented 8 months ago

Ok that's much more complicated than that. What they did should be forbidden and I already told them on multiple occasion. The refactor they make with this compulsory "project" parameter has made all existing pipelines to fail. This is very shortsighted but we cannot do anything about it.

2 main issues to overcome are:

Let me report here when I move forward with one of the items

12rambau commented 8 months ago

@dfguerrerom can you check a credential file from a user in SEPAL a copy/paste it here (anonymised) I want to check if the project is saved in the credentials or not with modern users. If it's not the case the whole SEPAL structure will be broken if yes we need to be clever on the credential discovery.

dfguerrerom commented 8 months ago

actually, yes, when an user, old or new, logs in using the SEPAL google authentication process, it will automatically save the project id selected on the UI in the credentials file, this file has the following structure:

{
    "access_token":"....",
    "access_token_expiry_date":"...", 
    "project_id":"something"
}

Note that SEPAL stores the project with the key project_id, while a local user that uses earthengine set_project "project_name" will add the key project to the credentials.

12rambau commented 8 months ago

ok perfect so that is exactly the same as in a service account token. I think we should rely on that. What if we start relying on geetools for this kind of stuff ? I'm going to revamp the authentication/Initto match this new structure and it can then be called in the init_ee decorator.

Without parameter it will try to reach the default credentials and read the project_id from the token

12rambau commented 8 months ago

I made a PR in this direction in geetools: https://github.com/gee-community/geetools/pull/240 I will use it here once it's released

dfguerrerom commented 7 months ago

@12rambau, note that as I mentioned before, SEPAL uses project_id, while the default earthengine set_project projectname will store the project in the project key.

12rambau commented 7 months ago

@cdanielw any reason why you are not relying on the default key generated by GEE ?

cdanielw commented 7 months ago

To my understanding, the default project id used when ee.initialize() is called, without explicitly specifying project, will be the legacy project. That one will stop working (if it hasn't already).

dfguerrerom commented 7 months ago

What I meant to say is that when an local user authenticates in earthengine, they have the possibility to earthengine set_project project_name. This project will be stored in the .config/earthengine/credentials file under the project key, SEPAL also stores this value in the credentials but in project_id key, not project. @12rambau I don't think there's a specific reason for that.

cdanielw commented 7 months ago

I didn't know there was a way to set that up. We can look into conforming to that.

12rambau commented 7 months ago

I think it's pretty recent, at start it was only in service accounts but now it's available for all account type as you need to set up the project when Initializing GEE.

12rambau commented 7 months ago

As this lib need to remain compatible with vanilla environments, I could you align on GEE behaviour ? I would like not to code custom behaviour that would only work on SEPAL.

dfguerrerom commented 7 months ago

As this lib need to remain compatible with vanilla environments, I could you align on GEE behaviour ? I would like not to code custom behaviour that would only work on SEPAL.

I think that's something we (sepal) can change at some point. @12rambau In the meantime I think we can rely on geetools to do the authentication, however I see that they (you) have implemented an extra auth method: from_user... I assume we'll have to implement that extra auth step in our init_ee but also keep supporting the current authentication method (for regular users) and also add from service account?

12rambau commented 7 months ago

from_user without argument is only relying on the default credentials so nothing to do from your side, as long as the project is setup as in a normal credential.

dfguerrerom commented 7 months ago

actually, the credentials stored in sepal are quite different to the ones you're expecting in https://github.com/gee-community/geetools/blob/47cb326c1a6bf4757756b3f10a89aece91a365ce/geetools/Initialize/__init__.py#L59, we have:

{"access_token":"","access_token_expiry_date":,"project_id":""}

but that's fine because we install our own fork that will authenticate using our custom credentials file.

question, Is there a reason why are you instantiating the Credentials class with the values from the credentials instead of just letting earthengine-api do that? I also saw the project arg is not passed to the Initialize method.

12rambau commented 7 months ago
  1. by doing that I'm agnostic to the account type (personnal or service account)
  2. As I try to provide support for multi-account I need to be able to use any file as source. Default will only use the one stored in .credentals
12rambau commented 6 months ago

Could you add the 2 "authentication" path we talked about when I was in Rome ? I think PR should be validated so you can work on main again

dfguerrerom commented 6 months ago

Could you add the 2 "authentication" path we talked about when I was in Rome ? I think PR should be validated so you can work on main again

@12rambau This PR contains what we discussed in Rome... if I'm remember correctly, it will work with both service account or individual gee accounts since the project is saved in the credentials file (?).

12rambau commented 6 months ago

Perfect, can you lint it and let's see how the tests behave from the CI/CD side

dfguerrerom commented 6 months ago

@12rambau, I think it fails because there is no a $secrets.EARTHENGINE_TOKEN in this project? to make it work we should have a EARTHENGINE_TOKEN with a project or EARTHENGINE_TOKEN with a EARTHENGINE_PROJECT venv.

12rambau commented 6 months ago

I added the appropriate parameters. Before checking "project" you need to check "poject_id" as it's the one set up in the service account profile keys. "project" is only existing in SEPAL.

dfguerrerom commented 6 months ago

project

Actually that's how it is written, but first, it will try with the EARTHENGINE_PROJECT which you introduced in pytest-gee.

It fails because with the google service account you cannot authenticate using that private key, we'll have to use the method described here: https://developers.google.com/earth-engine/guides/service_account#use-a-service-account-with-a-private-key (which is the one you use in pytest-gee).

The init_ee method should determine whether the credentials file is a service account or not (?).

12rambau commented 6 months ago

I guess yes but then it becomes more and more convoluted ;-) They could have come up with something unified....

dfguerrerom commented 6 months ago

haha it failed again. I don't really know what is the structure of the EARTHENGINE_TOKEN you have, but the one that I created has this type key...

12rambau commented 6 months ago

that is the one I store which a very default token from the latest GEE API version:

{
  "type": "service_account",
  "project_id": "sepal-ui",
  "private_key_id": "key_id",
  "private_key": "a key",
  "client_email": "a.fake@mail",
  "client_id": "id",
  "auth_uri": "a.fake.url",
  "token_uri": "a.fake.url",
  "auth_provider_x509_cert_url": "a.fake.url",
  "client_x509_cert_url": "a.fake.url",
  "universe_domain": "a.fake.domain"
}
dfguerrerom commented 5 months ago

closed in favor of https://github.com/12rambau/sepal_ui/pull/915

12rambau commented 5 months ago

can you remove the branch if it's not needed anymore ?