if the organization id is invalid, the current organization is set to nil (because find_by_id returns that).
This basically makes it look like return values in the "any context" mode when what happened in reality should have resulted in Organization Not Found error.
Any one have issues in me changing this logic to say "If you passed me an organization id and its invalid raise 404"
The way hammer works is not exactly like that - or at least I don't
get values from 'any context' when I pass an invalid organization.
$ hammer domain list --organization=asljksdaklsasadjkl
Error: organization not found
$ hammer domain list --organization-id=3123123123
404 Resource Not Found
The reason for this is that in the former case, hammer makes a scoped
search call searching for an organization with that name. In the
second it just goes to api/v2/organizations/id
If it can't find it, it doesn't proceed with searching for the
resource (domains in this case, proxy in yours) and returns a not
found. So the taxonomy_scope core should never get called in this
case.
Are you getting results for any context when you pass an organization
that doesn't exist? If so that's a security hole in my opinion, I
can't reproduce it but perhaps if you use the --debug flag and paste
the results we can figure it out.
···
On Tue, Feb 3, 2015 at 12:09 AM, Partha Aji wrote:
>
>
> Recently I was working on a bug where the following hammer call was passing with no errors and all smart proxies.
>
> hammer proxy list --organization-id="IM_NOT_REALLY_THERE"
>
> and foreman's smart proxy control follows the template used in other api controllers
>
> https://github.com/theforeman/foreman/blob/develop/app/controllers/api/v2/smart_proxies_controller.rb#L15
>
> With " include Api::TaxonomyScope" doing its org scoping.
>
> Anyway turns out that this line is the culprit
> https://github.com/theforeman/foreman/blob/develop/app/controllers/concerns/api/taxonomy_scope.rb#L14
>
> Organization.current ||= @organization = Organization.find_by_id(params[:organization_id])
>
> if the organization id is invalid, the current organization is set to nil (because find_by_id returns that).
>
> This basically makes it look like return values in the "any context" mode when what happened in reality should have resulted in Organization Not Found error.
>
> Any one have issues in me changing this logic to say "If you passed me an organization id and its invalid raise 404"
>
> Partha
>
> --
> You received this message because you are subscribed to the Google Groups "foreman-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to foreman-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
> From: "Daniel Lobato" <elobatocs@gmail.com>
> To: foreman-dev@googlegroups.com
> Sent: Thursday, February 5, 2015 5:15:28 AM
> Subject: Re: [foreman-dev] Question - Invalid Organization handling in foreman api
>
> The way hammer works is not exactly like that - or at least I don't
> get values from 'any context' when I pass an invalid organization.
>
> $ hammer domain list --organization=asljksdaklsasadjkl
> Error: organization not found
>
> $ hammer domain list --organization-id=3123123123
> 404 Resource Not Found
>
> The reason for this is that in the former case, hammer makes a scoped
> search call searching for an organization with that name. In the
> second it just goes to api/v2/organizations/id
> If it can't find it, it doesn't proceed with searching for the
> resource (domains in this case, proxy in yours) and returns a not
> found. So the taxonomy_scope core should never get called in this
> case.
>
> Are you getting results for any context when you pass an organization
> that doesn't exist? If so that's a security hole in my opinion, I
> can't reproduce it but perhaps if you use the --debug flag and paste
> the results we can figure it out.
In the case of domains I think the route raises a 404 when you provide an org id
[ INFO 2015-02-09 17:58:07 HammerCLIForeman::Domain::ListCommand] Called with options: {"option_organization_id"=>"IM_NOT_REALLY_THERE"}
[ INFO 2015-02-09 17:58:07 API] GET /api/organizations/IM_NOT_REALLY_THERE/domains
[DEBUG 2015-02-09 17:58:07 API] Params: {
"page" => 1,
"per_page" => 9999
}
GET /api/organizations/IM_NOT_REALLY_THERE/domains which raises a not found.
While in the case of smart proxies it is a different route with no org prefix
hammer proxy list --organization-id="IM_NOT_REALLY_THERE"
[ INFO 2015-02-09 18:01:25 HammerCLIForeman::SmartProxy::ListCommand] Called with options: {"option_organization_id"=>"IM_NOT_REALLY_THERE"}
[ INFO 2015-02-09 18:01:25 API] GET /api/smart_proxies
[DEBUG 2015-02-09 18:01:25 API] Params: {
"organization_id" => "IM_NOT_REALLY_THERE",
"page" => 1,
"per_page" => 9999
}
I guess 404 does not show up due to that and invalid organization id gets ignored.
···
----- Original Message -----
On Tue, Feb 3, 2015 at 12:09 AM, Partha Aji paji@redhat.com wrote:
Recently I was working on a bug where the following hammer call was passing
with no errors and all smart proxies.
hammer proxy list --organization-id=“IM_NOT_REALLY_THERE”
and foreman’s smart proxy control follows the template used in other api
controllers
if the organization id is invalid, the current organization is set to nil
(because find_by_id returns that).
This basically makes it look like return values in the “any context” mode
when what happened in reality should have resulted in Organization Not
Found error.
Any one have issues in me changing this logic to say “If you passed me an
organization id and its invalid raise 404”
