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

As a developer, I want to specify the project, WRES, in all WRDS service calls #282

Open
epag opened this issue Aug 21, 2024 · 16 comments
Open

Comments

@epag
Copy link
Collaborator

epag commented Aug 21, 2024


Author Name: Hank (Hank)
Original Redmine Issue: 97763, https://vlab.noaa.gov/redmine/issues/97763
Original Date: 2021-10-21


From Swagger, the field is @Proj@. For example,

https://nwcal-wrds.[host]/docs/location/v3.0/swagger/

From that documentation,

The project name that is accessing the API. Use _ in place of spaces. Using this parameter is necessary in order to streamline debugging any issues.

Examples:
proj=WRES

This needs to be added to +all+ calls to WRDS. It will become a required field, per MarkW.

When development begins, a checklist should be prepared with all the WRDS service interactions.

For now, the priority is high, but it will likely be upped to urgent in the near future. Thanks,

Hank


Redmine related issue(s): 100164


@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Jesse (Jesse)
Original Date: 2021-10-21T17:37:01Z


I did not comment during the meeting about this anti-feature of WRDS because it was neither here nor there. We could safely ignore it. But if it is going to become a breaking change, then I guess now is the time to push back on it.

The WRDS team already has the IP addresses of the caller. There is also already an HTTP header called @user-agent@ that can be used for this purpose. Both are already available to WRDS. They can use them today. Whether every team on the client side is actually identifying their applications using the @user-agent@ header is another question.

By attempting to require this parameter, the WRDS team can expect what Chris described in the meeting: either garbage or fraudulent values, not out of malice but out of an effort to get it to work, e.g. by people attempting to get work done.

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Hank (Hank)
Original Date: 2021-10-21T17:44:46Z


Jesse,

The WRDS team already has the IP addresses of the caller. There is also already an HTTP header called User-Agent that can be used for this purpose. Both are already available to WRDS. They can use them today. Whether every team on the client side is actually identifying their applications using the User-Agent header is another question.

Do we identify our applications using the User-Agent header? Just curiosity.

I've never been a big fan of this feature as it felt ripe for abuse, either accidental or purposeful (and not necessarily by the user). I may just be paranoid. :)

Hank

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Chris (Chris)
Original Date: 2021-10-21T18:12:50Z


Now, I'm not saying I do this, but it's currently possible to hit the service without even a cert (lol), so I don't see a lot of virtue from a user's perspective to stick anything but garbage in there.

I touched on this in the chat, but if they really have to have a tight understanding as to who is doing what, that's what API keys are for. I'd say that requiring an API key is the right way to do the wrong thing, so doing things in their proposed way is essentially the wrong way to do the wrong thing. This might make me a pedant, but you really shouldn't be able to modify data on a server with a @get@ request.

So, let's say someone enters @Proj=YoMomma@. What sort of trouble would they be in? Asking for a friend.

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Jesse (Jesse)
Original Date: 2021-10-21T18:30:07Z


Right now it's something like @Java/11.0.12/OkHttp-something@, in other words "obviously WRES" but I'm going to enable debug logging on OkHttp and see the exact string. We should append a WRES at least, and a version id if that's easy enough.

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Jesse (Jesse)
Original Date: 2021-10-21T19:07:13Z


So it was much more difficult than I hoped to enable logging of the actual headers (not just added headers as we do currently in @webclient@), went a different route and had WRES connect to a local nc server:

$ nc -l 5000
GET /test?endDT=2018-01-01T00%3A00%3A00Z&format=json&parameterCd=00060&sites=09165000&startDT=2017-01-01T00%3A00%3A01Z HTTP/1.1
Accept-Encoding: gzip
Host: localhost:5000
Connection: Keep-Alive
User-Agent: okhttp/4.10.0-RC1

So there's your answer: @okhttp/4.10.0-RC1@ but we can modify it to something else, which I think would be a fair resolution to this ticket.

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Jesse (Jesse)
Original Date: 2021-10-21T19:08:59Z


Without objection, setting it to @user-agent: YoMomma@

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Hank (Hank)
Original Date: 2021-10-21T19:11:06Z


Is it "YoMomma" or "YoMamma"? We should make sure of the spelling, first. :)

We'll have to get the WRDS team to buy in on doing something different. I'll bring it up at the next WRES/WRDS meeting in a couple weeks. If they require it, they require it, and we'll have no option but to play along. But if we can talk them into a different approach, "the right way to do the wrong thing" (what Jesse found or keys), or, better still, "just don't do the wrong thing", that would be an improvement.

Sounds like my suspicions, that this is a bad idea, were well founded even if I didn't have confidence in my opinion. Thanks,

Hank

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: James (James)
Original Date: 2021-10-21T19:12:52Z


I thought it was jo mamma.

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Chris (Chris)
Original Date: 2021-10-21T19:24:12Z


I've seen yo mama. Laugh Factory labels it as "Yo Momma", so there's that. We can always just have:

@user-agent: TeaIsJustBrownDirtWater@

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Jesse (Jesse)
Original Date: 2021-10-21T21:59:17Z


commit:626647401cf59f11e32f2093a658016191a22454 sets it to wres-io/version, e.g. @user-agent: wres-io/20211021-67c095a-dev@ from my machine. This actually might be helpful to see whether it is a clean build of wres-io (no @-dev@ dirty flag) versus a dirty build of wres-io (@-dev@ dirty flag).

As far as the parameter for the project, I believe our users have the ability to add the flag with existing mechanisms, do they not? Only in dev/test would we need to say "this is from the WRES project" right?

It's a bad-faith request, smells bad, looks bad... etc.

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Jesse (Jesse)
Original Date: 2021-10-21T22:00:33Z


In other words I acknowledge there is a difference between identifying the @user-agent@ (a piece of software) and identifying the project that is associated with the call. The latter is a runtime thing, the former is a compile-time thing.

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Jesse (Jesse)
Original Date: 2021-10-21T22:02:21Z


Oh, and before I forget, this only affects requests that go through @webclient@, so it wouldn't affect the requests that use the @cdm@ aka @netcdf-java@ library (those use something else under the hood, apache httpclient or otherwise).

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Hank (Hank)
Original Date: 2022-04-06T19:36:32Z


Looked briefly at a solution for the WRDS observed service. For that, I plan is to edit @WebSource.java@ to include the project field automatically when it builds WRDS URIs if the user does not already include it. That should impact the WRDS AHPS forecast and observation services. However, the NWM service appears to be handled elsewhere. I'll see if I can find it.

Hank

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Hank (Hank)
Original Date: 2022-04-06T19:44:09Z


Nevermind. The NWM WRDS services are handled in @WebSource.java@. So, to implement proj, we should be able to make the changes in @WebSource.java@ for all WRDS time series services.

I'll take a look at the WRDS location and threshold services later,

Hank

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Hank (Hank)
Original Date: 2022-04-07T12:59:14Z


commit:28e49118a3defded0f49c6b45586a791c7611014 has changes so that the proj field is used in all calls to WRDS time series services; i.e., the WRDS NWM, AHPS, and observation services.

Still need to resolve this for the location service calls and threshold service calls.

Thanks,

Hank

EDIT: Fixed the commit link

@epag
Copy link
Collaborator Author

epag commented Aug 21, 2024


Original Redmine Comment
Author Name: Hank (Hank)
Original Date: 2022-04-07T13:01:02Z


Also, note that the constant I defined to support that,

    private static final String DEFAULT_WRDS_PROJ = "WRES";
</code>

should perhaps be moved to a more central location in order to be used in changes for the location service, including thresholds.

Hank

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

1 participant