allowing scalar params to be null and still validate if they're not required

[seeekr]

this is important as e.g. AWS.config.credentials needs ProviderId to be null for google+ provider, as per current JS SDK docs.
I'm not 100% certain this is exactly how you'd make such a change but there definitely seems to be the need to enhance validation with instructions that deal with null values! Definitely for scalars, absolutely not sure about whether the other (composite) types need that at all.

[lsegal]

Unfortunately this breaks our existing test suite. Can you explain a little more about the issue you are seeing? The SDK should already handle passing null through to ProviderId.

[lsegal]

@seeekr thanks for the pull request illustrating the deficiency in parameter validation. I've taken your code and added some extra functionality to deal with null values passing through to serialization, and also supporting nulls on structs/maps/lists. In short, passing a null should now behave as if the property was not set, which is the intention of the guide documentation when it suggests setting ProviderId to null.

Note that the current workaround would be to omit ProviderId entirely for Google+, or set it to undefined instead. We can also consider updating the documentation, if that makes it less confusing.

[seeekr]

@lsegal awesome! I hadn't had the chance yet to get back to the issue and it's great to see that you've figured it out without further input from me! What you're saying sounds great!
re: leaving out ProviderId for Google+ - I had tried that but it didn't work and at that time I thought it might be that the server side API maybe defaulted to using the Amazon (or FB, but Amazon seemed more reasonable) auth mechanism and I thought that's why it had failed. BUT after trying it with the properly validating ProviderId: null I still couldn't get it to work (and couldn't figure out why) so it probably did default to using Google+, but maybe you can double-check that.

re docs: Yes, I do think that until there is a new RC release of the API the docs should be updated to reflect that ProviderId: null will not work and instead to leave it out for Google+, maybe ideally with a note highlighted in red so it's clear what would be the "proper" way of doing it and that just until RC10 (or whatever release is next) the workaround is to do it this way.

[seeekr]

@lsegal and wow, I just had a look at the commit - it would have taken me ages to get into the code well enough to be able to make (suggest) the appropriate changes myself :) thanks!

[lsegal]

@seeekr keep in mind that for Google+ your WebIdentityToken has to use the authResponse.id_token, not the access_token like in other identity providers. Here's an example from fine-uploader: https://github.com/Widen/fine-uploader-examples/blob/master/src/s3-no-server/google-auth.js#L19-L22 -- that might be the error you were having.

[seeekr]

@lsegal oh god, I totally used access_token. I just double-checked what the (otherwise pretty good and easy to follow, good work there!) AWS JS SDK docs are saying and there's no mention of this, it just says ACCESS_TOKEN. I feel like it would remove a lot of potential for frustration for newbie "3rd party auth" (or w/e we can call this) users like me if there was a note in the docs about this particular thing with using Google+ auth. It didn't even occur to me to look for something other than access_token since the name fits so perfectly with what the docs are saying.

[lsegal]

I agree that we could be doing a better job documenting these integration points between identity providers. Improving this getting started experience is on our roadmap. Thanks for the feedback!

[seeekr]

OK, so I had to go back and try that. Yes, there is definitely a difference between using ProviderId: null and leaving it out. And actually... it now works if I don't specify that field. If I set it to null (and it puts it into the request like that) I get an HTTP 403 error.

re: getting started experience - cool, it's pretty good already though :) Docs are very easy to follow, even though some of the process is a bit tedious and requires lots of steps in different UIs.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.