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

💩 🔧 Enable suspending data collection for certain subgroups #1016

Merged
merged 2 commits into from
Feb 3, 2025

Conversation

shankari
Copy link
Contributor

@shankari shankari commented Feb 3, 2025

This implements a hack for e-mission/e-mission-docs#1104
The request in the issue was to implement the ability to suspend and resume
data collection.

As a first step for implementing this, we:

  • add a new section to the opcode in the dynamic config that lists the
    suspended subgroups, and
  • skip all calls to usercache/put for users that are in suspended subgroup

This is a hack because the real fix is to stop data collection on the phone.

But even with this implementation:

  • I have copied over the implementation of getSubgroupFromToken from the phone and manually ported it over to python. Instead, we should have all the implementations for parsing the opcode in e-mission-common and just use them here
  • ideally, we would check for the subgroup being suspended in the putIntoCache method. But, by design, we only see the opcode in the auth module; the rest of the code works only with UUIDs. We should figure out a way to return the non-random parts of the opcode (program and subgroup) as a context so we can do additional validation downstream if needed. We can return this as a context variable, or store it into the profile and have the code check it from there.

@JGreenlee @TeachMeTW we should discuss if we want to continue with this hack on the server, or just go to the principled fix, in which case, most of these 💩 changes can be reverted.

Before this, the dynamic config was read in `resolve_auth` and only used to
choose the authentication method if it was dynamic. However, as we make more
parts of the server also controlled via the dynamic config, we will want to
make the dynamic config more broadly available.

This change pulls out the dynamic config into a separate function, invokes it
before resolving the auth, and passes it in the `resolve_auth` function.

A potential cleanup would be to pull it up even further (into
`emission.core.dynamic_config`, similar to
`emission/core/backwards_compat_config.py`) so that it could be used more
easily in other modules as well.

Testing done:

auth_mode = skip

```
2025-02-02 12:43:15,973:DEBUG:8313653312:Using auth method skip
2025-02-02 12:45:30,066:DEBUG:6329921536:START POST /profile/create
2025-02-02 12:45:30,068:DEBUG:6329921536:Called createUserProfile
2025-02-02 12:45:30,074:DEBUG:6329921536:methodName = skip, returning <class 'emission.net.aut
h.skip.SkipMethod'>
2025-02-02 12:45:30,075:DEBUG:6329921536:Using the skip method to verify id token nrelop_dev-e
mulator-study_default_123 of length 37
2025-02-02 12:45:30,075:DEBUG:6329921536:userEmail = nrelop_dev-emulator-study_default_123
...
2025-02-02 12:45:30,103:DEBUG:6329921536:END POST /profile/create  0.0375370979309082
```

auth_mode = dynamic, but token not in token list

```
ValueError: Invalid token nrelop_dev-emulator-study_default_123, not found in list of length 0

2025-02-02 12:49:37,234:DEBUG:8313653312:auth_method is dynamic, using dynamic config to find
the actual auth method
2025-02-02 12:49:37,234:DEBUG:8313653312:is a program set auth_method to token_list
2025-02-02 12:49:37,235:DEBUG:8313653312:Using auth method token_list
2025-02-02 12:50:07,735:DEBUG:6353514496:START POST /profile/create
2025-02-02 12:50:07,737:DEBUG:6353514496:Called createUserProfile
et.auth.token_list.TokenListMethod'>
2025-02-02 12:50:07,744:DEBUG:6370340864:methodName = token_list, returning <class 'emission.n
et.auth.token_list.TokenListMethod'>
2025-02-02 12:50:07,746:DEBUG:6353514496:Using the TokenListMethod to verify id token nrelop_d
ev-emulator-study_default_123 of length 37 against list []...
2025-02-02 12:50:07,749:DEBUG:6353514496:END POST /profile/create  0.015098094940185547
```

auth_mode = dynamic, token in token list

```
./e-mission-py.bash bin/auth/insert_tokens.py --single nrelop_dev-emulator-study_default_123
Config file not found, returning a copy of the environment variables instead...
Retrieved config: {'DB_HOST': None, 'DB_RESULT_LIMIT': None}
URL not formatted, defaulting to "Stage_database"
Connecting to database URL localhost

2025-02-02 12:53:31,260:DEBUG:8313653312:attempting to resolve auth_method
2025-02-02 12:53:31,261:DEBUG:8313653312:STUDY_CONFIG is stage-program
2025-02-02 12:53:31,261:DEBUG:8313653312:About to download config from https://raw.githubuserc
ontent.com/e-mission/nrel-openpath-deploy-configs/main/configs/stage-program.nrel-op.json
2025-02-02 12:53:31,368:DEBUG:8313653312:Successfully downloaded config with version 1 for Sta
ging environment for testing programs only and data collection URL https://openpath-stage.nrel
.gov/api/
2025-02-02 12:53:31,369:DEBUG:8313653312:auth_method is dynamic, using dynamic config to find the actual auth method
2025-02-02 12:53:31,369:DEBUG:8313653312:is a program set auth_method to token_list
2025-02-02 12:53:31,369:DEBUG:8313653312:Using auth method token_list
2025-02-02 12:54:15,996:DEBUG:13186920448:START POST /profile/create
2025-02-02 12:54:15,997:DEBUG:13186920448:Called createUserProfile
2025-02-02 12:54:16,001:DEBUG:13186920448:methodName = token_list, returning <class 'emission.net.auth.token_list.TokenListMethod'>
2025-02-02 12:54:16,011:DEBUG:13203746816:Using the TokenListMethod to verify id token nrelop_dev-emulator-study_default_123 of length 37 against list ['nrelop_dev-emulator-study_default_123']...
2025-02-02 12:54:16,011:DEBUG:13203746816:Found match for token nrelop_dev-emulator-study_default_123 of length 37
2025-02-02 12:54:16,014:DEBUG:13203746816:retUUID = eb4a7aae-f2d4-4480-ba85-c568a45591b5
2025-02-02 12:54:16,016:DEBUG:13186920448:Looked up user = <emission.core.wrapper.user.User object at 0x3105a3820>
2025-02-02 12:54:16,016:DEBUG:13186920448:Returning result {'uuid': 'eb4a7aae-f2d4-4480-ba85-c568a45591b5'}
2025-02-02 12:54:16,017:DEBUG:13186920448:END POST /profile/create  0.020717620849609375
```
@shankari
Copy link
Contributor Author

shankari commented Feb 3, 2025

Testing done:

After changing the default config

STUDY_CONFIG = os.getenv('STUDY_CONFIG', "dev-emulator-study")

and changing the dev-emulator-study config to have the new-style opcode with
a suspended subgroup

    "opcode": {
        "autogen": true,
        "subgroups": [
          "test",
          "default"
        ],
        "suspended_subgroups": ["default"]
    },

The suspended subgroup skipped all put messages

2025-02-02 18:57:35,723:DEBUG:6363951104:START POST /usercache/put
2025-02-02 18:57:35,724:DEBUG:6363951104:Called userCache.put
2025-02-02 18:57:35,742:DEBUG:6363951104:subgroup default found in list ['test', 'default']
2025-02-02 18:57:35,743:INFO:6363951104:Received put message for subgroup default in suspended
_subgroups=['default'], returning uuid = None
2025-02-02 18:57:35,743:DEBUG:6363951104:retUUID = None
2025-02-02 18:57:35,743:DEBUG:6380777472:START POST /usercache/get
2025-02-02 18:57:35,743:DEBUG:6363951104:END POST /usercache/put  0.023092985153198242

The non-suspended subgroup used the default implementation of put

2025-02-02 19:15:42,723:DEBUG:13035925504:START POST /usercache/put
2025-02-02 19:15:42,726:DEBUG:13035925504:Called userCache.put
2025-02-02 19:15:42,750:DEBUG:13035925504:subgroup test found in list ['test', 'default']
2025-02-02 19:15:42,759:DEBUG:13035925504:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_time, write_ts = 1738551626.711804 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a034de46756f8244ede842'), 'ok': 1.0, 'updatedExisting': False}
2025-02-02 19:15:42,761:DEBUG:13035925504:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_time, write_ts = 1738551626.71355 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a034de46756f8244ede844'), 'ok': 1.0, 'updatedExisting': False}

After modifying the config to remove suspended_subgroups (default case)

    "opcode": {
        "autogen": true,
        "subgroups": [
          "test",
          "default"
        ],
    },
2025-02-02 21:08:26,082:DEBUG:13052751872:START POST /usercache/put
2025-02-02 21:08:26,082:DEBUG:13052751872:Called userCache.put
2025-02-02 21:08:26,088:DEBUG:13052751872:subgroup default found in list ['test', 'default']
2025-02-02 21:08:26,088:DEBUG:13052751872:methodName = skip, returning <class 'emission.net.auth.skip.SkipMethod'>
2025-02-02 21:08:26,088:DEBUG:13052751872:Using the skip method to verify id token nrelop_dev-emulator-study_default_123 of length 37

2025-02-02 21:08:26,110:DEBUG:13052751872:Updated result for user = eb4a7aae-f2d4-4480-ba85-c568a45591b5, key = background/location, write_ts = 1738559116.0185199 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a04f4a46756f8244ee3a88'), 'ok': 1.0, 'updatedExisting': False}
2025-02-02 21:08:26,112:DEBUG:13052751872:Updated result for user = eb4a7aae-f2d4-4480-ba85-c568a45591b5, key = background/filtered_location, write_ts = 1738559116.020091 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a04f4a46756f8244ee3a8a'), 'ok': 1.0, 'updatedExisting': False}
...
2025-02-02 21:08:26,930:DEBUG:13052751872:Updated result for user = eb4a7aae-f2d4-4480-ba85-c568a45591b5, key = stats/client_nav_event, write_ts = 1738559306.0461679 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a04f4a46756f8244ee3da8'), 'ok': 1.0, 'updatedExisting': False}
2025-02-02 21:08:26,930:DEBUG:13052751872:END POST /usercache/put eb4a7aae-f2d4-4480-ba85-c568a45591b5 0.849830150604248

@shankari shankari force-pushed the suspend_certain_subgroups branch from 6c51207 to bc0aeb1 Compare February 3, 2025 06:16
This implements a hack for e-mission/e-mission-docs#1104
The request in the issue was to implement the ability to suspend and resume
data collection.

As a first step for implementing this, we:
- add a new section to the `opcode` in the dynamic config that lists the
  suspended subgroups, and
- skip all calls to `usercache/put` for users that are in suspended subgroup

This is a hack because the real fix is to stop data collection on the phone.

But even with this implementation:
- I have copied over the implementation of `getSubgroupFromToken` from the
  phone and manually ported it over to python. Instead, we should have all the
  implementations for parsing the opcode in `e-mission-common` and just use them here
- ideally, we would check for the subgroup being suspended in the
  `putIntoCache` method. But, by design, we only see the opcode in the auth
  module; the rest of the code works only with UUIDs. We should figure out a
  way to return the non-random parts of the opcode (program and subgroup) as a
  context so we can do additional validation downstream if needed. We can
  return this as a context variable, or store it into the profile and have the
  code check it from there.

Testing done:

After changing the default config

```
STUDY_CONFIG = os.getenv('STUDY_CONFIG', "dev-emulator-study")
```

and changing the `dev-emulator-study` config to have the new-style opcode with
a suspended subgroup

```
    "opcode": {
        "autogen": true,
        "subgroups": [
          "test",
          "default"
        ],
        "suspended_subgroups": ["default"]
    },
```

The suspended subgroup skipped all put messages

```
2025-02-02 18:57:35,723:DEBUG:6363951104:START POST /usercache/put
2025-02-02 18:57:35,724:DEBUG:6363951104:Called userCache.put
2025-02-02 18:57:35,742:DEBUG:6363951104:subgroup default found in list ['test', 'default']
2025-02-02 18:57:35,743:INFO:6363951104:Received put message for subgroup default in suspended
_subgroups=['default'], returning uuid = None
2025-02-02 18:57:35,743:DEBUG:6363951104:retUUID = None
2025-02-02 18:57:35,743:DEBUG:6380777472:START POST /usercache/get
2025-02-02 18:57:35,743:DEBUG:6363951104:END POST /usercache/put  0.023092985153198242
```

The non-suspended subgroup used the default implementation of put

```
2025-02-02 19:15:42,723:DEBUG:13035925504:START POST /usercache/put
2025-02-02 19:15:42,726:DEBUG:13035925504:Called userCache.put
2025-02-02 19:15:42,750:DEBUG:13035925504:subgroup test found in list ['test', 'default']
2025-02-02 19:15:42,759:DEBUG:13035925504:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_time, write_ts = 1738551626.711804 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a034de46756f8244ede842'), 'ok': 1.0, 'updatedExisting': False}
2025-02-02 19:15:42,761:DEBUG:13035925504:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_time, write_ts = 1738551626.71355 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a034de46756f8244ede844'), 'ok': 1.0, 'updatedExisting': False}
```

After modifying the config to remove suspended_subgroups (default case)

```
    "opcode": {
        "autogen": true,
        "subgroups": [
          "test",
          "default"
        ],
    },
```

```
2025-02-02 21:08:26,082:DEBUG:13052751872:START POST /usercache/put
2025-02-02 21:08:26,082:DEBUG:13052751872:Called userCache.put
2025-02-02 21:08:26,088:DEBUG:13052751872:subgroup default found in list ['test', 'default']
2025-02-02 21:08:26,088:DEBUG:13052751872:methodName = skip, returning <class 'emission.net.auth.skip.SkipMethod'>
2025-02-02 21:08:26,088:DEBUG:13052751872:Using the skip method to verify id token nrelop_dev-emulator-study_default_123 of length 37

2025-02-02 21:08:26,110:DEBUG:13052751872:Updated result for user = eb4a7aae-f2d4-4480-ba85-c568a45591b5, key = background/location, write_ts = 1738559116.0185199 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a04f4a46756f8244ee3a88'), 'ok': 1.0, 'updatedExisting': False}
2025-02-02 21:08:26,112:DEBUG:13052751872:Updated result for user = eb4a7aae-f2d4-4480-ba85-c568a45591b5, key = background/filtered_location, write_ts = 1738559116.020091 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a04f4a46756f8244ee3a8a'), 'ok': 1.0, 'updatedExisting': False}
...
2025-02-02 21:08:26,930:DEBUG:13052751872:Updated result for user = eb4a7aae-f2d4-4480-ba85-c568a45591b5, key = stats/client_nav_event, write_ts = 1738559306.0461679 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a04f4a46756f8244ee3da8'), 'ok': 1.0, 'updatedExisting': False}
2025-02-02 21:08:26,930:DEBUG:13052751872:END POST /usercache/put eb4a7aae-f2d4-4480-ba85-c568a45591b5 0.849830150604248
```
@shankari shankari force-pushed the suspend_certain_subgroups branch from bc0aeb1 to cbb29e4 Compare February 3, 2025 06:36
@shankari
Copy link
Contributor Author

shankari commented Feb 3, 2025

@mattwigway the hack has been merged now. It will be on staging for a couple of days, and then we will move it to production, likely by mid-week. After it is in production, you can choose to edit the config and suspend the subgroup at any time.

@shankari shankari merged commit aaedcd3 into e-mission:master Feb 3, 2025
5 checks passed
@mattwigway
Copy link

mattwigway commented Feb 3, 2025 via email

@shankari
Copy link
Contributor Author

shankari commented Feb 7, 2025

We tested this on staging by:

  • adding a new temp subgroup
  • configuring two test phones to use the temp subgroup
  • collecting data for a day (trips were uploaded and processed)
  • suspending the temp subgroup and restarting the server to pick up the change
  • collecting data for a day (upload was ignored)
2025-02-07 04:25:19,567:INFO:139885876868672:Received put message for subgroup temp in suspended_subgroups=['temp'], returning uuid = None
2025-02-07 04:27:43,377:INFO:139885834909248:Received put message for subgroup temp in suspended_subgroups=['temp'], returning uuid = None
2025-02-07 06:00:01,237:INFO:139886261958208:Received put message for subgroup temp in suspended_subgroups=['temp'], returning uuid = None
2025-02-07 06:55:25,869:INFO:139885824419392:Received put message for subgroup temp in suspended_subgroups=['temp'], returning uuid = None
  • BUT argh! the change returns a 403 error to the phone, so the data keeps accumulating there.
  • I need to change the server to return a 200; otherwise, this is just the same as shutting down the server.
13817,1738898733.391,2025-02-06T19:25:33.391000-08:00,BuiltinUserCache : Result count = 278
13833,1738898733.429,2025-02-06T19:25:33.429000-08:00,"ServerSyncAdapter : IO Error java.net.UnknownHostException: Unable to resolve host ""openpath-stage.nrel.gov"": No address associated with hostname while posting converted trips to JSON"
13835,1738898733.431,2025-02-06T19:25:33.431000-08:00,BuiltinUserCache : Added value for key stats/client_error at time 1.738898733431E9
13836,1738898733.434,2025-02-06T19:25:33.434000-08:00,"ServerSyncAdapter : Push complete, now pulling"

14449,1738902319.436,2025-02-06T20:25:19.436000-08:00,BuiltinUserCache : Result count = 278
14454,1738902319.736,2025-02-06T20:25:19.736000-08:00,CommunicationHelper : Got response org.apache.http.message.BasicHttpResponse@8ce8b76 with statusHTTP/1.1 403 Forbidden
14455,1738902319.755,2025-02-06T20:25:19.755000-08:00,ServerSyncAdapter : IO Error java.io.IOException while posting converted trips to JSON
14457,1738902319.759,2025-02-06T20:25:19.759000-08:00,"ServerSyncAdapter : Push complete, now pulling"

14768,1738902463.316,2025-02-06T20:27:43.316000-08:00,BuiltinUserCache : Result count = 356
14786,1738902463.551,2025-02-06T20:27:43.551000-08:00,CommunicationHelper : Got response org.apache.http.message.BasicHttpResponse@7c4d0d4 with statusHTTP/1.1 403 Forbidden
14787,1738902463.569,2025-02-06T20:27:43.569000-08:00,ServerSyncAdapter : IO Error java.io.IOException while posting converted trips to JSON
14789,1738902463.572,2025-02-06T20:27:43.572000-08:00,"ServerSyncAdapter : Push complete, now pulling"


14832,1738908000.968,2025-02-06T22:00:00.968000-08:00,BuiltinUserCache : Result count = 356
14837,1738908001.459,2025-02-06T22:00:01.459000-08:00,CommunicationHelper : Got response org.apache.http.message.BasicHttpResponse@193ea46 with statusHTTP/1.1 403 Forbidden
14838,1738908001.462,2025-02-06T22:00:01.462000-08:00,ServerSyncAdapter : IO Error java.io.IOException while posting converted trips to JSON
14840,1738908001.464,2025-02-06T22:00:01.464000-08:00,"ServerSyncAdapter : Push complete, now pulling"

@shankari
Copy link
Contributor Author

shankari commented Feb 7, 2025

The reason we are seeing the 403 error is because returning a None UUID errors out with a 403 even before the check in the usercache/put

        retUUID = enaa.getUUID(dynamic_config, request, auth_method, inHeader)
        logging.debug("retUUID = %s" % retUUID)
        if retUUID is None:
           raise HTTPError(403, "token is valid, but no account found for user")
        return retUUID

There are other ways to fix this (e.g. if the call is usercache/put, then don't return the 403), but instead of building a hack on top of a hack, I'm going to return the auth context instead and put the check into usercache/put as it should be.

shankari added a commit to shankari/e-mission-server that referenced this pull request Feb 7, 2025
This revises the initial implementation in
e-mission#1016
of a hack for e-mission/e-mission-docs#1104

The request in the issue was to implement the ability to suspend and resume
data collection.

The previous implementation checked for suspended subgroups in the auth method,
and used the `request.path` to determine whether it was a `usercache/put` call or not
This was needed because, by design, we only see the opcode in the auth module;
the rest of the code works only with UUIDs

The problem with that approach was that if the subgroup was suspended, we
returned a UUID of None, which triggered a 403 error, which, in turn, caused
the phone app to detect a failed call, so it did not delete the entries locally.

e-mission#1016 (comment)
e-mission#1016 (comment)

We fix this by returning the non-random parts of the opcode (at least the
subgroup) in a context, and perform the validation downstream (in the
`putIntoCache` method). So if the subgroup is suspended, `putIntoCache` just
returns early instead of storing the values.

Note that this is also a bit of a hack, given that the way we handle the
dynamic auth right now is to reset the global `auth_method`!! So we can't use
a check for `auth_method == 'dynamic'` anywhere outside `resolve_auth` and have
to use the existence of the dynamic config as the check for whether the auth
method was dynamic or not

Testing done:
- TestWebserver.py passes
- TestAuthSelection.py does not need any changes since we made sure to be
  backwards compatible

When trying to push to a suspended subgroup, we get

```
2025-02-07 11:10:26,127:DEBUG:6362165248:START POST /usercache/put
2025-02-07 11:10:26,127:DEBUG:6362165248:Called userCache.put
2025-02-07 11:10:26,189:DEBUG:6362165248:methodName = skip, returning <class 'emission.net.au
th.skip.SkipMethod'>
2025-02-07 11:10:26,189:DEBUG:6362165248:Using the skip method to verify id token nrelop_dev-
emulator-study_default_123 of length 37
2025-02-07 11:10:26,190:DEBUG:6362165248:subgroup default found in list ['test', 'default']
2025-02-07 11:10:26,190:DEBUG:6362165248:retContext = {'subgroup': 'default', 'user_id': UUID
('eb4a7aae-f2d4-4480-ba85-c568a45591b5')}
2025-02-07 11:10:26,191:DEBUG:6362165248:user_uuid {'subgroup': 'default', 'user_id': UUID('e
b4a7aae-f2d4-4480-ba85-c568a45591b5')}
2025-02-07 11:10:26,191:DEBUG:6362165248:suspended_subgroups=['default']
2025-02-07 11:10:26,191:INFO:6362165248:Received put message for subgroup default in suspende
d_subgroups=['default'], ignoring
2025-02-07 11:10:26,191:DEBUG:6362165248:END POST /usercache/put eb4a7aae-f2d4-4480-ba85-c568
```

When trying to push to a non-suspended subgroup, we get

```
2025-02-07 11:26:13,627:DEBUG:12901707776:START POST /usercache/put
2025-02-07 11:26:13,628:DEBUG:12901707776:Called userCache.put
2025-02-07 11:26:13,631:DEBUG:12901707776:methodName = skip, returning <class 'emission.net.auth.skip.SkipMethod'>
2025-02-07 11:26:13,631:DEBUG:12901707776:Using the skip method to verify id token nrelop_dev-emulator-study_test_123 of length 34
2025-02-07 11:26:13,634:DEBUG:12901707776:subgroup test found in list ['test', 'default']
2025-02-07 11:26:13,634:DEBUG:12901707776:retContext = {'subgroup': 'test', 'user_id': UUID('feb70456-abf4-444b-8848-1515fc3470cf')}
2025-02-07 11:26:13,634:DEBUG:12901707776:user_uuid {'subgroup': 'test', 'user_id': UUID('feb70456-abf4-444b-8848-1515fc3470cf')}
2025-02-07 11:26:13,634:DEBUG:12901707776:suspended_subgroups=['default']
2025-02-07 11:26:13,644:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_time, write_ts = 1738956332.243175 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f024f6'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,646:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_time, write_ts = 1738956332.246808 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f024f8'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,857:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = statemachine/transition, write_ts = 1738956373.598038 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f025cd'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,858:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = background/battery, write_ts = 1738956373.5991821 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f025cf'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,859:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_nav_event, write_ts = 1738956373.607026 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f025d1'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,860:DEBUG:12901707776:END POST /usercache/put feb70456-abf4-444b-8848-1515fc3470cf 0.2325270175933838
```
shankari added a commit to shankari/e-mission-server that referenced this pull request Feb 7, 2025
This revises the initial implementation in
e-mission#1016
of a hack for e-mission/e-mission-docs#1104

The request in the issue was to implement the ability to suspend and resume
data collection.

The previous implementation checked for suspended subgroups in the auth method,
and used the `request.path` to determine whether it was a `usercache/put` call or not
This was needed because, by design, we only see the opcode in the auth module;
the rest of the code works only with UUIDs

The problem with that approach was that if the subgroup was suspended, we
returned a UUID of None, which triggered a 403 error, which, in turn, caused
the phone app to detect a failed call, so it did not delete the entries locally.

e-mission#1016 (comment)
e-mission#1016 (comment)

We fix this by returning the non-random parts of the opcode (at least the
subgroup) in a context, and perform the validation downstream (in the
`putIntoCache` method). So if the subgroup is suspended, `putIntoCache` just
returns early instead of storing the values.

Note that this is also a bit of a hack, given that the way we handle the
dynamic auth right now is to reset the global `auth_method`!! So we can't use
a check for `auth_method == 'dynamic'` anywhere outside `resolve_auth` and have
to use the existence of the dynamic config as the check for whether the auth
method was dynamic or not

Testing done:
- TestWebserver.py passes
- TestAuthSelection.py does not need any changes since we made sure to be
  backwards compatible

When trying to push to a suspended subgroup, we get

```
2025-02-07 11:10:26,127:DEBUG:6362165248:START POST /usercache/put
2025-02-07 11:10:26,127:DEBUG:6362165248:Called userCache.put
2025-02-07 11:10:26,189:DEBUG:6362165248:methodName = skip, returning <class 'emission.net.au
th.skip.SkipMethod'>
2025-02-07 11:10:26,189:DEBUG:6362165248:Using the skip method to verify id token nrelop_dev-
emulator-study_default_123 of length 37
2025-02-07 11:10:26,190:DEBUG:6362165248:subgroup default found in list ['test', 'default']
2025-02-07 11:10:26,190:DEBUG:6362165248:retContext = {'subgroup': 'default', 'user_id': UUID
('eb4a7aae-f2d4-4480-ba85-c568a45591b5')}
2025-02-07 11:10:26,191:DEBUG:6362165248:user_uuid {'subgroup': 'default', 'user_id': UUID('e
b4a7aae-f2d4-4480-ba85-c568a45591b5')}
2025-02-07 11:10:26,191:DEBUG:6362165248:suspended_subgroups=['default']
2025-02-07 11:10:26,191:INFO:6362165248:Received put message for subgroup default in suspende
d_subgroups=['default'], ignoring
2025-02-07 11:10:26,191:DEBUG:6362165248:END POST /usercache/put eb4a7aae-f2d4-4480-ba85-c568
```

When trying to push to a non-suspended subgroup, we get

```
2025-02-07 11:26:13,627:DEBUG:12901707776:START POST /usercache/put
2025-02-07 11:26:13,628:DEBUG:12901707776:Called userCache.put
2025-02-07 11:26:13,631:DEBUG:12901707776:methodName = skip, returning <class 'emission.net.auth.skip.SkipMethod'>
2025-02-07 11:26:13,631:DEBUG:12901707776:Using the skip method to verify id token nrelop_dev-emulator-study_test_123 of length 34
2025-02-07 11:26:13,634:DEBUG:12901707776:subgroup test found in list ['test', 'default']
2025-02-07 11:26:13,634:DEBUG:12901707776:retContext = {'subgroup': 'test', 'user_id': UUID('feb70456-abf4-444b-8848-1515fc3470cf')}
2025-02-07 11:26:13,634:DEBUG:12901707776:user_uuid {'subgroup': 'test', 'user_id': UUID('feb70456-abf4-444b-8848-1515fc3470cf')}
2025-02-07 11:26:13,634:DEBUG:12901707776:suspended_subgroups=['default']
2025-02-07 11:26:13,644:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_time, write_ts = 1738956332.243175 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f024f6'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,646:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_time, write_ts = 1738956332.246808 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f024f8'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,857:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = statemachine/transition, write_ts = 1738956373.598038 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f025cd'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,858:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = background/battery, write_ts = 1738956373.5991821 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f025cf'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,859:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_nav_event, write_ts = 1738956373.607026 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f025d1'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,860:DEBUG:12901707776:END POST /usercache/put feb70456-abf4-444b-8848-1515fc3470cf 0.2325270175933838
```
@shankari
Copy link
Contributor Author

shankari commented Feb 7, 2025

Fixed the 403 error in #1018 and rebased to remove the changes here since they were 💩 anyway

shankari added a commit to shankari/e-mission-server that referenced this pull request Feb 7, 2025
This revises the initial implementation in
e-mission#1016
of a hack for e-mission/e-mission-docs#1104

The request in the issue was to implement the ability to suspend and resume
data collection.

The previous implementation checked for suspended subgroups in the auth method,
and used the `request.path` to determine whether it was a `usercache/put` call or not
This was needed because, by design, we only see the opcode in the auth module;
the rest of the code works only with UUIDs

The problem with that approach was that if the subgroup was suspended, we
returned a UUID of None, which triggered a 403 error, which, in turn, caused
the phone app to detect a failed call, so it did not delete the entries locally.

e-mission#1016 (comment)
e-mission#1016 (comment)

We fix this by returning the non-random parts of the opcode (at least the
subgroup) in a context, and perform the validation downstream (in the
`putIntoCache` method). So if the subgroup is suspended, `putIntoCache` just
returns early instead of storing the values.

Note that this is also a bit of a hack, given that the way we handle the
dynamic auth right now is to reset the global `auth_method`!! So we can't use
a check for `auth_method == 'dynamic'` anywhere outside `resolve_auth` and have
to use the existence of the dynamic config as the check for whether the auth
method was dynamic or not

Testing done:
- TestWebserver.py passes
- TestAuthSelection.py does not need any changes since we made sure to be
  backwards compatible

When trying to push to a suspended subgroup, we get

```
2025-02-07 11:10:26,127:DEBUG:6362165248:START POST /usercache/put
2025-02-07 11:10:26,127:DEBUG:6362165248:Called userCache.put
2025-02-07 11:10:26,189:DEBUG:6362165248:methodName = skip, returning <class 'emission.net.au
th.skip.SkipMethod'>
2025-02-07 11:10:26,189:DEBUG:6362165248:Using the skip method to verify id token nrelop_dev-
emulator-study_default_123 of length 37
2025-02-07 11:10:26,190:DEBUG:6362165248:subgroup default found in list ['test', 'default']
2025-02-07 11:10:26,190:DEBUG:6362165248:retContext = {'subgroup': 'default', 'user_id': UUID
('eb4a7aae-f2d4-4480-ba85-c568a45591b5')}
2025-02-07 11:10:26,191:DEBUG:6362165248:user_uuid {'subgroup': 'default', 'user_id': UUID('e
b4a7aae-f2d4-4480-ba85-c568a45591b5')}
2025-02-07 11:10:26,191:DEBUG:6362165248:suspended_subgroups=['default']
2025-02-07 11:10:26,191:INFO:6362165248:Received put message for subgroup default in suspende
d_subgroups=['default'], ignoring
2025-02-07 11:10:26,191:DEBUG:6362165248:END POST /usercache/put eb4a7aae-f2d4-4480-ba85-c568
```

When trying to push to a non-suspended subgroup, we get

```
2025-02-07 11:26:13,627:DEBUG:12901707776:START POST /usercache/put
2025-02-07 11:26:13,628:DEBUG:12901707776:Called userCache.put
2025-02-07 11:26:13,631:DEBUG:12901707776:methodName = skip, returning <class 'emission.net.auth.skip.SkipMethod'>
2025-02-07 11:26:13,631:DEBUG:12901707776:Using the skip method to verify id token nrelop_dev-emulator-study_test_123 of length 34
2025-02-07 11:26:13,634:DEBUG:12901707776:subgroup test found in list ['test', 'default']
2025-02-07 11:26:13,634:DEBUG:12901707776:retContext = {'subgroup': 'test', 'user_id': UUID('feb70456-abf4-444b-8848-1515fc3470cf')}
2025-02-07 11:26:13,634:DEBUG:12901707776:user_uuid {'subgroup': 'test', 'user_id': UUID('feb70456-abf4-444b-8848-1515fc3470cf')}
2025-02-07 11:26:13,634:DEBUG:12901707776:suspended_subgroups=['default']
2025-02-07 11:26:13,644:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_time, write_ts = 1738956332.243175 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f024f6'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,646:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_time, write_ts = 1738956332.246808 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f024f8'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,857:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = statemachine/transition, write_ts = 1738956373.598038 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f025cd'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,858:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = background/battery, write_ts = 1738956373.5991821 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f025cf'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,859:DEBUG:12901707776:Updated result for user = feb70456-abf4-444b-8848-1515fc3470cf, key = stats/client_nav_event, write_ts = 1738956373.607026 = {'n': 1, 'nModified': 0, 'upserted': ObjectId('67a65e5546756f8244f025d1'), 'ok': 1.0, 'updatedExisting': False}
2025-02-07 11:26:13,860:DEBUG:12901707776:END POST /usercache/put feb70456-abf4-444b-8848-1515fc3470cf 0.2325270175933838
```
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

Successfully merging this pull request may close these issues.

2 participants