这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@rikinsk-zz
Copy link

Description

Fixes bug in permission builder. Columns of object relationships in a non-public schema weren't showing up.

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

Solution and Design

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

@rikinsk-zz rikinsk-zz requested a review from wawhal February 5, 2019 14:25
@hasura-bot
Copy link
Contributor

Review app for commit a6b70e5 deployed to Heroku: https://hge-ci-pull-1562.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1562-a6b70e5

Copy link
Contributor

@wawhal wawhal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested. LGTM.

@wawhal wawhal added k/bug Something isn't working s/ok-to-merge Status: This pull request can be merged to master c/console Related to console labels Feb 5, 2019
@shahidhk
Copy link
Member

shahidhk commented Feb 6, 2019

Need one more review, @praveenweb @karthikvt26 ?

@hasura-bot
Copy link
Contributor

Review app for commit f1b2542 deployed to Heroku: https://hge-ci-pull-1562.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1562-f1b2542

@rikinsk-zz rikinsk-zz merged commit 4bf9d19 into hasura:master Feb 6, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-1562.herokuapp.com is deleted

@rikinsk-zz rikinsk-zz deleted the console-permissions branch February 6, 2019 07:57
hasura-bot pushed a commit that referenced this pull request Jan 27, 2025
### What

Add support for environment variables in permission filters by
introducing a new `ValueFromEnv` variant. This enhancement allows users
to reference environment variables in permission rules and value
expressions, similar to how `sessionVariable` and `literal` values work.

Key changes:
- Added `ValueFromEnv` variant to `ValueExpression` and
`ValueExpressionOrPredicate`
- Updated environment variables handling to support JSON values instead
of strings (we need this as the literal values can be something other
than `String` as well
- Added changes to JSON schemas for the types as well

### How

1. Modified `crates/open-dds/src/permissions.rs`:
- Split `ValueExpression` and `ValueExpressionOrPredicate` into base
enums and their schema implementation counterparts
(`ValueExpressionImpl` and `ValueExpressionOrPredicateImpl`)
- Added `ValueFromEnv` variant to support environment variable
references
   - Implemented proper JSON schema generation through `OpenDd` trait

2. Updated environment variables handling in
`build/environment_variables.rs`:
- Changed value type from `String` to `serde_json::Value` to support
various value types
   - Updated injection and restoration logic to handle JSON values

Users can now reference environment variables in their permissions
metadata:
```json
{
  "role": "artist1",
  "select": {
    "filter": {
      "fieldComparison": {
        "field": "ArtistId",
        "operator": "_eq",
        "value": {
          "valueFromEnv": "ENV_VAR1"
        }
      }
    }
  }
}
```

### Internal details

V3_GIT_ORIGIN_REV_ID: 2dc2da8b7266d9c68887ac27990ba8bbea17166b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/console Related to console k/bug Something isn't working s/ok-to-merge Status: This pull request can be merged to master

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants