Introduces CloudTenancyMixin to fix RBAC for cloud_tenant based models #13535

rwsu · 2017-01-17T06:48:33Z

RBAC is broken for models based on cloud_tenant instead of tenant.
Currently a cloud tenant user can see all cloud_tenant based
objects. This patches introduces a CloudTenancyMixin that a
cloud_tenant model can include for RBAC to work correctly.

CloudTenancyMixin is similar to TenancyMixin in that it defines
methods used by Rbac.Filterer.scope_targets to filter results
so that users only sees objects that belong to their tenant
groups.

CloudVolume now includes CloudTenancyMixin which fixes issue

Additional models using cloud_tenant will be updated in subsequent
patches.

Fixes

RBAC is not working correctly for models with :cloud_tenant #13343 (comment)

Steps for Testing/QA [Optional]

In OpenStack Dashboard,

create two projects, project1 and project2, add the admin user as a admin to both projects (otherwise you won't be able to setup the groups users in ManageIQ as the admin user)
create one volume in project1
create one volume in project2

Verify you have Tenant Mapping Enabled set to True for your OpenStack Cloud Provider. If not recreate it.

Under Configuration > Access Control > Groups

create an admin group for project/tenant project1 and has role EvmRole-super_administrator
create an admin group for project/tenant project2 and has role EvmRole-super_administrator

Under Configuration > Access Control > Users

create a user project1-admin who belongs to project1 admin group and has EvmRole-super_administrator role
create a user project2-admin who belongs to project2 admin group and has EvmRole-super_administrator role

Login as project1-admin, navigate to Storage > Block Storage > Volumes, and verify you only see volumes belonging to project1
Login as project2-admin, navigate to Storage > Block Storage > Volumes, and verify you only see volumes belonging to project2

lpichler · 2017-01-17T14:03:22Z

Thanks @rwsu! I have some questions now.

What CloudVolumes are filtered when Tenant#source_tenant is nil ?
What CloudVolumes are filtered in cloud volumes when some comes from provider without cloud tenant mapping and other from provider with cloud tenant mapping ?

rwsu · 2017-01-18T00:54:22Z

@lpichler Good points.

if source_tenant is nil, then no records are returned. In which situation(s) would source_tenant be nil?
I will need to tweak this to enable scope_by_tenant only if the tenant mapping is enabled for the cloud provider.

lpichler · 2017-01-18T16:27:58Z

if source_tenant is nil, then no records are returned. In which situation(s) would source_tenant be nil?
Basically when you are not using cloud tenant mapping (the checkbox)
and then Tenant#source_tenant is not populated.

So I guess when there are nil it should list all objects(CloudVolumes) as it was so far without cloud tenant mapping.
But there is also case when I refresh provider with cloud tenant mapping then
Tenant#source_tenant is populated but then when I switch off cloud tenant mapping then
Tenant#source_tenant is still populated but you are not using cloud tenant mapping so it should also filter all objects. (I think this is correct behaviour because then you can add other projects in openstack and then refresh in Miq and then new tenants will be have nils in source_tenant so it seems confusable what to filter)
So my opinion is
cloud tenant mapping is on => filter objects based on mapped cloud tenants
cloud tenant mapping is off => display all objects (or how it is now)

I will need to tweak this to enable scope_by_tenant only if the tenant mapping is enabled for the cloud provider.

yes 👍 basically it is similar what I am saying in 1. :)

tzumainn · 2017-01-25T16:11:26Z

@rwsu just to check, would this update also fix cloud tenant - flavor mapping?

rwsu · 2017-02-05T21:25:19Z

Upon further analysis, this approach I've been taking only works if there is only one cloud provider or in single object views. When the view is against all objects and there are multiple cloud providers that have tenant_mapping_enabled as both true and false, then this fails.

Consider this example:
provider 1, tenant_mapping_enabled = true, use CloudTenancyMixin (scope joins to cloud_tenant and tenants)
provider 2, tenant_mapping_enabled = false, use TenancyMixin

In this example, we can't apply the CloudTenancyMixin logic in Filterer.scope_to_target because it only applies to objects in provider 1 and not provider 2.

For this to work correctly, there needs to be two separate queries one where we search for tenant_mapping_enabled = true and apply CloudTenancyMixin. And a second query where tenant_mapping_enabled = false and TenancyMixin is applied. Then the results of the two queries are unioned.

A single scope, an ActiveRecord::Relation, is defined in Filterer.search. We could modify Filterer search to run the two queries and combine their results. But I'm not sure if this is the best approach. What do you think? @lpichler @gtanzillo

gtanzillo · 2017-02-07T16:44:04Z

@rwsu I think for the case where cloud tenant mapping is off, you can still scope to the owing tenant of the parent EMS through TenancyMixin#ems_tenant.

gtanzillo · 2017-02-07T16:37:12Z

app/models/mixins/cloud_tenancy_mixin.rb

+      {"tenants" => {:id => tenant_ids}}
+    end
+
+    def joins_clause


I don't see where this is getting called.

I omitted a file. Please see update.

@gtanzillo Replying to your previous comment. Yes we can scope to the owning tenant of the parent EMS through TenancyMixin#ems_tenant. But the difficulty is in how to construct the scope when there are multiple cloud providers, and when one of the cloud provider as tenant mapping disabled and another had it enabled. In my comment above, I tried to explain that it would require two separate queries. The scope created in Filterer.search is a single ActiveRecord::Relation. So is creating multiple queries in Filterer.search ok? Or do we need to do some additional refactoring to accommodate cloud tenants?

lpichler · 2017-02-09T16:00:22Z

@rwsu yes such query can be complicated. I can look on it. But I realized one thing.
Does your intention in this PR count with adding CloudTenant to RBAC ?
Maybe we should start with this first.

rwsu · 2017-02-15T04:56:06Z

@lpichler I think I figured it out. Multiple queries weren't required after all, just needed to think a bit more about which conditionals to add to the query. Please take a look and see if it makes sense.

I also had to add a change to CloudManager to propagate tenant_mapping_enabled to the storage providers. Without it, the value wasn't being set and the tenant_id_clause from cloud tenancy mixin would not work correctly.

lpichler

Hi @rwsu, I am sorry for delay generally I like these changes, good job 👍

I left here some comments and I found case https://github.com/ManageIQ/manageiq/pull/13535/files/28d1cc4e42c90c90466228f15aba8bc39ef142d4#r104653606 which need to be fixed.
I stated some suggestion but if you have other idea how to do query, let me know.

Also I would be nice to have some tests in filterer_specs
at least for cases:

When you have some cloud volumes some of them belong to logged user's tenant some of them no.
2.Same as 1 + when you have some cloud volumes in manager with tenant mapping and some in manager without tenant mapping

thanks!

lpichler · 2017-03-07T09:35:03Z

app/models/manageiq/providers/openstack/cloud_manager.rb

@@ -66,6 +66,15 @@ def ensure_swift_manager
    true
  end

+  after_save :save_on_other_managers


what about to use
delegate :tenant_mapping_enabled, :to => :parent_manager
for example on class
StorageManager < ManageIQ::Providers::BaseManager

ah, I guess no, now I understand why we need it

lpichler · 2017-03-07T11:20:08Z

app/models/manageiq/providers/openstack/cloud_manager.rb

+  after_save :save_on_other_managers
+
+  def save_on_other_managers
+    storage_managers.each do |manager|


this can be:
storage_managers.update_all(:tenant_mapping_enabled => tenant_mapping_enabled)

lpichler · 2017-03-07T12:21:14Z

app/models/mixins/cloud_tenancy_mixin.rb

+      ["(tenants.id IN (?) AND ext_management_systems.tenant_mapping_enabled IS TRUE) OR ext_management_systems.tenant_mapping_enabled IS FALSE", tenant_ids]
+    end
+
+    def tenant_joins_clause(scope)


I am not sure about this.

I have CloudVolumes and CloudVolume called CV1 belongs to TCV1 tenant, I have also user ICV1 for tenant TCV1
CV1 has manager with tenant_mapping_enabled and there are other manager for rest CloudVolumes with disabled tenant_mapping_enabled.

When I will run Rbac for user ICV1
it returns me only CV1 and not not other cloud volumes and I think that (query looks to me that is should works in this way) that it should
I thought that we could do something this:

a = CloudVolume.includes(:cloud_tenant => :source_tenant).includes(:ext_management_system).where(:tenants => { :id => [1000000000010]}, :ext_management_systems => { :tenant_mapping_enabled => true} ) b = CloudVolume.includes(:cloud_tenant => :source_tenant).includes(:ext_management_system).where(:ext_management_systems => { :tenant_mapping_enabled => [false, nil]} ) a.or(b)

but there is error where wheres clauses have to have same structure:

Relation passed to #or must be structurally compatible. Incompatible values: [:references]

Maybe we can take it that we can return just ids

a.ids | b.ids
and apply it to query.

What do you think about this ?

I found the issue. In this PR the joins_clause uses joins where as the second PR in the cloud tenant series, e471dbb, switched over to using includes. I switched the second patch to using includes, because some tests were failing and I had thought it was an issue with missing attributes in the test data. But I now understand that if the cloud_tenant_mapping is not enabled, then an inner join to the cloud_tenants table would always return 0 results, so an outer join (includes) is actually necessary. As far as I can tell, the outer join is only required against cloud_tenant and not ext_management_system.

I will revise this PR and rebase the second in this series.

rwsu · 2017-03-08T23:26:02Z

@lpichler Hi please have a look again. Is the update_all offense found by rubocop a concern?

lpichler · 2017-03-15T11:25:14Z

spec/models/mixins/cloud_tenancy_mixin_spec.rb

+    let(:tenant_users) { FactoryGirl.create(:miq_user_role, :name => "tenant-users") }
+    let(:tenant_group) { FactoryGirl.create(:miq_group, :miq_user_role => tenant_users, :tenant => tenant) }
+
+    it "finds correct tenant id clause for regular tenants" do


lpichler · 2017-03-15T11:30:26Z

app/models/mixins/cloud_tenancy_mixin.rb

+    include TenancyCommonMixin
+
+    def tenant_id_clause_format(tenant_ids)
+      ["(tenants.id IN (?) AND ext_management_systems.tenant_mapping_enabled IS TRUE) OR ext_management_systems.tenant_mapping_enabled IS FALSE", tenant_ids]


we have default value for ext_management_systems.tenant_mapping_enabled NULL
so can be included also NULL here as well as you have FALSE ?

Good point. I will add a check for NULL.

lpichler · 2017-03-15T11:31:30Z

app/models/mixins/cloud_tenancy_mixin.rb

+    end
+  end
+
+  def tenant


we are using it somewhere? where ?
if so shouldn't it be cloud_tenant.try(:source_tenant)?

Ok, this appears to be not needed. Will remove.

lpichler · 2017-03-15T11:48:39Z

lib/rbac/filterer.rb

@@ -424,6 +424,10 @@ def scope_to_tenant(scope, user, miq_group)
      user_or_group = user || miq_group
      tenant_id_clause = klass.tenant_id_clause(user_or_group)

+      if klass.respond_to?(:tenant_joins_clause)


@rwsu I am just thinking about.
this is supporting for supporting cloud_tenancy_mixin and tenancy mixin at the same time ?

Should not it be exclusive ? only either cloud_tenancy_mixin or only either tenancy_mixin ?
If so I would create in cloud tenant mixin
method :scope_by_cloud_tenant?
and in filterer move out this condition from scope_to_tenant
and add elsif branch for in scope_targets method

if klass.respond_to?(:scope_by_tenant?) && klass.scope_by_tenant? scope = scope_to_tenant(scope, user, miq_group) elsif klass.try(:scope_by_cloud_tenant?) scope = klass.tenant_joins_clause(scope) end

What do you think ? I think we should separate this two things in RBAC if it will be work.

Good question. I don't think they should be exclusive. I'm just modifying the current flow of adding additional scopes for tenants. Currently, the scope_to_tenant adds where clauses. My change adds a new step to the flow by adding additional joins if one is desired.

Both TenancyMixin and CloudTenancyMixin adds tenant_id_clauses. So :scope_by_cloud_tenant? can't just add the joins_clause. It needs to add the joins_clause and the tenant_id_clause which is currently done in scope_to_tenant.

I understand that you would like to see a clearer separation by using :scope_by_tenant and :scope_by_cloud_tenant. But it is not clear to me if that is necessary here or how to achieve that cleanly without duplicating code.

diff --git a/app/models/mixins/cloud_tenancy_mixin.rb b/app/models/mixins/cloud_tenancy_mixin.rb index 8b6b8f90dd..c35e1d5559 100644 --- a/app/models/mixins/cloud_tenancy_mixin.rb +++ b/app/models/mixins/cloud_tenancy_mixin.rb @@ -2,6 +2,10 @@ module CloudTenancyMixin extend ActiveSupport::Concern module ClassMethods + def scope_by_cloud_tenant? + true + end + include TenancyCommonMixin def tenant_id_clause_format(tenant_ids) diff --git a/app/models/mixins/tenancy_common_mixin.rb b/app/models/mixins/tenancy_common_mixin.rb index c4a8aa74f3..7707e9d346 100644 --- a/app/models/mixins/tenancy_common_mixin.rb +++ b/app/models/mixins/tenancy_common_mixin.rb @@ -1,8 +1,4 @@ module TenancyCommonMixin diff --git a/app/models/mixins/cloud_tenancy_mixin.rb b/app/models/mixins/cloud_tenancy_mixin.rb index 8b6b8f90dd..c35e1d5559 100644 --- a/app/models/mixins/cloud_tenancy_mixin.rb +++ b/app/models/mixins/cloud_tenancy_mixin.rb @@ -2,6 +2,10 @@ module CloudTenancyMixin extend ActiveSupport::Concern module ClassMethods + def scope_by_cloud_tenant? + true + end + include TenancyCommonMixin def tenant_id_clause_format(tenant_ids) diff --git a/app/models/mixins/tenancy_common_mixin.rb b/app/models/mixins/tenancy_common_mixin.rb index c4a8aa74f3..7707e9d346 100644 --- a/app/models/mixins/tenancy_common_mixin.rb +++ b/app/models/mixins/tenancy_common_mixin.rb @@ -1,8 +1,4 @@ module TenancyCommonMixin - def scope_by_tenant? - true - end - def accessible_tenant_ids(user_or_group, strategy) tenant = user_or_group.try(:current_tenant) return [] if tenant.nil? || tenant.root? diff --git a/app/models/mixins/tenancy_mixin.rb b/app/models/mixins/tenancy_mixin.rb index 37681143cb..e10594aa05 100644 --- a/app/models/mixins/tenancy_mixin.rb +++ b/app/models/mixins/tenancy_mixin.rb @@ -6,6 +6,10 @@ module TenancyMixin end module ClassMethods + def scope_by_tenant? + true + end + include TenancyCommonMixin end diff --git a/lib/rbac/filterer.rb b/lib/rbac/filterer.rb index 463ffc818c..ae903962de 100644 --- a/lib/rbac/filterer.rb +++ b/lib/rbac/filterer.rb @@ -427,14 +427,15 @@ module Rbac klass = scope.respond_to?(:klass) ? scope.klass : scope user_or_group = user || miq_group tenant_id_clause = klass.tenant_id_clause(user_or_group) - - if klass.respond_to?(:tenant_joins_clause) - scope = klass.tenant_joins_clause(scope) - end - tenant_id_clause ? scope.where(tenant_id_clause) : scope end + def scope_to_cloud_tenant(scope, user, miq_group) + klass = scope.respond_to?(:klass) ? scope.klass : scope + tenant_id_clause = klass.tenant_id_clause(user || miq_group) + klass.tenant_joins_clause(scope).where(tenant_id_clause) + end + ## # Main scoping method # @@ -444,6 +445,8 @@ module Rbac # TENANT_ACCESS_STRATEGY are a consolidated list of them. if klass.respond_to?(:scope_by_tenant?) && klass.scope_by_tenant? scope = scope_to_tenant(scope, user, miq_group) + elsif klass.respond_to?(:scope_by_cloud_tenant?) + scope = scope_to_cloud_tenant(scope, user, miq_group) end if apply_rbac_directly?(klass)

@rwsu I was thinking about it like this ^^, your code nicely adding cloud tenant scoping to the tenant scoping.

but I think that should be exclusive because
why we need cloud tenant scoping ? it is because we cannot use classic tenant scoping on cloud objects because there is missing relation to tenant.
for this case
there is tenant mapping and relation is established thru cloud_class -> cloud_tenant -> tenant
as you know.

So this is the reason why it have to be exclusive.

When in future(this is not likely) will cloud objects get the relation directly to tenant, then there will be used classic tenancy mixin.

and my patch as you said yes some duplicated code but I believe we can clean up later, (maybe it can be moved to tenancy mixins )
there is important to see structure scoping and what why it is happening in rbac.

lpichler · 2017-03-15T11:49:52Z

app/models/mixins/tenancy_common_mixin.rb

+    tenant_id_clause_format(tenant_ids)
+  end
+
+  def tenant_id_clause_format(tenant_ids)


just question, why it is moved to method ? is called on other place then on line 17?

To share lines 14-15 between TenancyCommonMixin and CloudTenancyMixin. CloudTenancyMixin has a different set of conditionals for the tenant_id_clause.

lpichler

@rwsu rubocop I believe that it is ok here.

I tested it and it is working 👏

I stated some comments.

RBAC is broken for models based on cloud_tenant instead of tenant. Currently a cloud tenant user can see all cloud_tenant based objects. This patches introduces a CloudTenancyMixin that a cloud_tenant model can include for RBAC to work correctly. CloudTenancyMixin is similar to TenancyMixin in that it defines methods used by Rbac.Filterer.scope_targets to filter results so that users only sees objects that belong to their tenant groups. CloudVolume now includes CloudTenancyMixin which fixes issue Additional models using cloud_tenant will be updated in subsequent patches.

Cinder and Swift Managers do not have cloud_tenant_mapping set. The OpenStack CloudManager now sets each storage provider's cloud_tenant_mapping whenever there is an update. This is required because CloudTenancyMixin checks the storage provider's cloud_tenant_mapping value, and not the cloud provider's value.

Thanks to lpichler.

The INNER JOIN was too restrictive. If the tenant mapping is not enabled, then the cloud_tenant to tenant relation is not established, causing the query to not match any rows. Some test data do not set ext_management_system for cloud objects. So we need to relax the INNER JOIN on ext_management_system to OUTER JOIN. Also added additional tests to validate cloud tenant filtering works correctly.

FALSE doesn't imply NULL.

As suggested by lpilcher.

This clarifies which mixin is being used, and establishes a clearer separation of logic being applied. As suggested by lpilcher.

miq-bot · 2017-03-17T05:32:30Z

Checked commits rwsu/manageiq@fc32e7e~...33a3327 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
8 files checked, 1 offense detected

app/models/manageiq/providers/openstack/cloud_manager.rb

❗ - Line 72, Col 22 - Rails/SkipsModelValidations - Avoid using update_all because it skips validations.

lpichler

👍 Looks good to me @gtanzillo

gtanzillo

Looks good 👍

chessbyte assigned gtanzillo Jan 17, 2017

chessbyte added enhancement core/rbac labels Jan 17, 2017

gtanzillo requested a review from lpichler January 17, 2017 14:53

rwsu force-pushed the cloud_tenancy_mixin branch from 7653881 to f6de90d Compare January 17, 2017 20:07

gtanzillo reviewed Feb 7, 2017

View reviewed changes

rwsu force-pushed the cloud_tenancy_mixin branch from f6de90d to 00798b0 Compare February 8, 2017 05:09

rwsu force-pushed the cloud_tenancy_mixin branch 2 times, most recently from 72dc282 to 538bec8 Compare February 15, 2017 04:25

rwsu force-pushed the cloud_tenancy_mixin branch 3 times, most recently from aa7805a to e74b91f Compare February 22, 2017 19:39

rwsu mentioned this pull request Feb 22, 2017

Enable cloud_tenant based RBAC for additional models #14036

Merged

rwsu force-pushed the cloud_tenancy_mixin branch from e74b91f to 28d1cc4 Compare February 24, 2017 04:45

lpichler suggested changes Mar 7, 2017

View reviewed changes

rwsu force-pushed the cloud_tenancy_mixin branch from e45d045 to 40c21e8 Compare March 8, 2017 22:59

lpichler reviewed Mar 15, 2017

View reviewed changes

rwsu force-pushed the cloud_tenancy_mixin branch from 40c21e8 to a1f7aab Compare March 16, 2017 02:41

rwsu force-pushed the cloud_tenancy_mixin branch from a1f7aab to d1ffa5f Compare March 16, 2017 03:31

rwsu force-pushed the cloud_tenancy_mixin branch 2 times, most recently from 6c7e95f to 32bc5e2 Compare March 16, 2017 07:05

rwsu added 8 commits March 16, 2017 21:42

Code simplification

381a0ba

Thanks to lpichler.

Need to check tenant_mapping_enabled for FALSE and NULL separately.

3a86234

FALSE doesn't imply NULL.

Clean up tests

4e9b56e

As suggested by lpilcher.

Remove unneeded tenant method

530df45

Make TenancyMixin and CloudTenancyMixin mutually exclusive

33a3327

This clarifies which mixin is being used, and establishes a clearer separation of logic being applied. As suggested by lpilcher.

rwsu force-pushed the cloud_tenancy_mixin branch from 32bc5e2 to 33a3327 Compare March 17, 2017 05:26

lpichler approved these changes Mar 17, 2017

View reviewed changes

gtanzillo approved these changes Mar 19, 2017

View reviewed changes

gtanzillo added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 19, 2017

gtanzillo merged commit a684fcf into ManageIQ:master Mar 19, 2017

lpichler mentioned this pull request Mar 31, 2017

[EUWE] Tenancy for CloudTenant #14593

Merged

lpichler mentioned this pull request Jul 12, 2019

Add orchestration stacks to RBAC #18308

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduces CloudTenancyMixin to fix RBAC for cloud_tenant based models #13535

Introduces CloudTenancyMixin to fix RBAC for cloud_tenant based models #13535

rwsu commented Jan 17, 2017

lpichler commented Jan 17, 2017 •

edited

Loading

rwsu commented Jan 18, 2017

lpichler commented Jan 18, 2017

tzumainn commented Jan 25, 2017

rwsu commented Feb 5, 2017

gtanzillo commented Feb 7, 2017

gtanzillo Feb 7, 2017

rwsu Feb 8, 2017

rwsu Feb 8, 2017

lpichler commented Feb 9, 2017

rwsu commented Feb 15, 2017

lpichler left a comment

lpichler Mar 7, 2017

lpichler Mar 7, 2017

lpichler Mar 7, 2017

lpichler Mar 7, 2017

rwsu Mar 7, 2017

rwsu commented Mar 8, 2017

lpichler Mar 15, 2017

lpichler Mar 15, 2017

rwsu Mar 15, 2017

lpichler Mar 15, 2017 •

edited

Loading

rwsu Mar 16, 2017

lpichler Mar 15, 2017 •

edited

Loading

rwsu Mar 15, 2017

lpichler Mar 16, 2017

lpichler Mar 16, 2017

lpichler Mar 15, 2017

rwsu Mar 15, 2017

lpichler left a comment

miq-bot commented Mar 17, 2017

lpichler left a comment

gtanzillo left a comment

Introduces CloudTenancyMixin to fix RBAC for cloud_tenant based models #13535

Introduces CloudTenancyMixin to fix RBAC for cloud_tenant based models #13535

Conversation

rwsu commented Jan 17, 2017

Fixes

Steps for Testing/QA [Optional]

lpichler commented Jan 17, 2017 • edited Loading

rwsu commented Jan 18, 2017

lpichler commented Jan 18, 2017

tzumainn commented Jan 25, 2017

rwsu commented Feb 5, 2017

gtanzillo commented Feb 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpichler commented Feb 9, 2017

rwsu commented Feb 15, 2017

lpichler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rwsu commented Mar 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpichler Mar 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpichler Mar 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpichler left a comment

Choose a reason for hiding this comment

miq-bot commented Mar 17, 2017

lpichler left a comment

Choose a reason for hiding this comment

gtanzillo left a comment

Choose a reason for hiding this comment

lpichler commented Jan 17, 2017 •

edited

Loading

lpichler Mar 15, 2017 •

edited

Loading

lpichler Mar 15, 2017 •

edited

Loading