-
Notifications
You must be signed in to change notification settings - Fork 1
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
Comments
Original Redmine Comment 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. |
Original Redmine Comment Jesse,
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 |
Original Redmine Comment 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. |
Original Redmine Comment 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. |
Original Redmine Comment 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:
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. |
Original Redmine Comment Without objection, setting it to @user-agent: YoMomma@ |
Original Redmine Comment 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 |
Original Redmine Comment I thought it was jo mamma. |
Original Redmine Comment I've seen yo mama. Laugh Factory labels it as "Yo Momma", so there's that. We can always just have: @user-agent: TeaIsJustBrownDirtWater@ |
Original Redmine Comment 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. |
Original Redmine Comment 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. |
Original Redmine Comment 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). |
Original Redmine Comment 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 |
Original Redmine Comment 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 |
Original Redmine Comment 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 |
Original Redmine Comment Also, note that the constant I defined to support that,
should perhaps be moved to a more central location in order to be used in changes for the location service, including thresholds. Hank |
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,
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
The text was updated successfully, but these errors were encountered: