-
Notifications
You must be signed in to change notification settings - Fork 601
Ability to inherit auth credentials #917
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
base: main
Are you sure you want to change the base?
Conversation
β¦edentials from environments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements an auth inheritance feature that allows HTTP requests to inherit authentication credentials from their active environment, similar to Postman/Insomnia behavior. This eliminates repetitive authentication configuration across multiple requests by centralizing auth at the environment level.
Key Changes:
- Added
AuthInheritanceTypeenum withnoneandenvironmentvalues to control inheritance behavior - Extended
EnvironmentModelwithdefaultAuthModelfield to store environment-level authentication - Added
authInheritanceTypefield toHttpRequestModelto specify whether a request inherits auth - Implemented auth resolution logic in
CollectionStateNotifierthat applies environment auth when inheritance is enabled - Created new UI components including environment auth editor tab and inheritance type dropdown
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
test/models/environment_models_test.dart |
Added test cases for EnvironmentModel with defaultAuthModel field, covering copyWith, toJson, and fromJson operations |
test/models/environment_models.dart |
Added test data for EnvironmentModel with auth (environmentModelWithAuth) and corresponding JSON representation |
packages/better_networking/lib/models/http_request_model.dart |
Added authInheritanceType field to enable requests to specify their auth inheritance behavior |
packages/better_networking/lib/consts.dart |
Defined AuthInheritanceType enum with none and environment values for controlling auth inheritance |
packages/apidash_core/lib/models/environment_model.g.dart |
Updated generated JSON serialization code to handle defaultAuthModel field |
packages/apidash_core/lib/models/environment_model.dart |
Added defaultAuthModel field to store default authentication configuration at environment level |
lib/utils/envvar_utils.dart |
Added placeholder code block for auth inheritance (actual logic implemented in CollectionStateNotifier) |
lib/screens/home_page/editor_pane/details_card/request_pane/request_auth.dart |
Integrated auth inheritance type selection into request auth UI, passing callbacks to update state |
lib/screens/envvar/environment_editor.dart |
Restructured UI to use TabBar with Variables and Auth tabs for environment configuration |
lib/screens/envvar/environment_auth_editor.dart |
New component for managing environment-level default authentication configuration |
lib/screens/common_widgets/auth/auth_page.dart |
Enhanced to support auth inheritance dropdown and conditionally display auth fields based on inheritance type |
lib/providers/environment_providers.dart |
Updated updateEnvironment method to support updating defaultAuthModel field |
lib/providers/collection_providers.dart |
Implemented core auth inheritance logic that substitutes environment auth when inheritance is enabled |
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| final updatedEnvironment = environment.copyWith( | ||
| name: name ?? environment.name, | ||
| values: values ?? environment.values, | ||
| defaultAuthModel: defaultAuthModel ?? environment.defaultAuthModel, |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The defaultAuthModel update logic has a potential issue. When defaultAuthModel is null in the function arguments but exists in the current environment, it will preserve the existing value due to the ?? operator. However, if you want to explicitly remove the defaultAuthModel (set it to null), this logic prevents that.
Consider using a different pattern to allow explicit null values, such as using an Optional<T> wrapper or checking if the parameter was provided.
| // Auth inheritance selection | ||
| if (!readOnly) ...[ | ||
| ADPopupMenu<AuthInheritanceType>( | ||
| value: (authInheritanceType ?? AuthInheritanceType.none).displayType, | ||
| values: AuthInheritanceType.values | ||
| .map((type) => (type, type.displayType)) | ||
| .toList(), | ||
| tooltip: "Select auth inheritance type", | ||
| isOutlined: true, | ||
| onChanged: onChangedAuthInheritanceType, | ||
| ), | ||
| const SizedBox(height: 16), | ||
| ], |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The auth inheritance dropdown is shown for all non-readonly scenarios, but it should only be shown in the request auth editor, not in the environment auth editor. Environment-level auth is the source of inherited auth, so showing an inheritance dropdown there doesn't make logical sense.
Consider adding a parameter to conditionally show the inheritance dropdown, or pass a flag to disable it when used in environment context.
| // Handle auth inheritance | ||
| HttpRequestModel processedRequestModel = httpRequestModel; | ||
| if (httpRequestModel.authInheritanceType == AuthInheritanceType.environment && | ||
| activeEnvId != null && | ||
| environments != null && | ||
| environments[activeEnvId] != null) { | ||
|
|
||
| final environment = environments[activeEnvId]!; | ||
| if (environment.defaultAuthModel != null) { | ||
| processedRequestModel = httpRequestModel.copyWith( | ||
| authModel: environment.defaultAuthModel, | ||
| ); | ||
| } | ||
| } |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The auth inheritance logic in getSubstitutedHttpRequestModel lacks test coverage. Given the complexity of this feature and that it's a core part of the PR, tests should verify:
- Auth is inherited when
authInheritanceTypeisenvironmentand environment hasdefaultAuthModel - Request-specific auth is used when
authInheritanceTypeisnoneornull - Behavior when environment doesn't have
defaultAuthModel - Behavior when no active environment is selected
| final environmentModelcopyWith = | ||
| environmentModel.copyWith(defaultAuthModel: newAuthModel); |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable name environmentModelcopyWith should follow camelCase convention and have proper spacing. It should be environmentModelCopyWith (capital C in CopyWith).
This naming issue also appears on line 11 where it's first declared correctly but here it lacks proper casing.
| // Handle auth inheritance | ||
| AuthModel? finalAuthModel = httpRequestModel.authModel; | ||
| if (httpRequestModel.authInheritanceType == AuthInheritanceType.environment && | ||
| activeEnvironmentId != null) { | ||
| // Get the environment model to access its default auth | ||
| // This would require access to the full environment models, not just variables | ||
| // For now, we'll handle this in the request processing where we have full environment access | ||
| } |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code block is incomplete and serves no purpose - it only contains a comment stating that functionality will be handled elsewhere. This is dead code that adds confusion.
Either remove this entire auth inheritance handling block (lines 74-81) since the actual implementation is in CollectionStateNotifier.getSubstitutedHttpRequestModel(), or add a comment explaining why this placeholder exists.
| List<NameValueModel>? headers, | ||
| List<NameValueModel>? params, | ||
| @Default(AuthModel(type: APIAuthType.none)) AuthModel? authModel, | ||
| AuthInheritanceType? authInheritanceType, // Add auth inheritance type |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The authInheritanceType field is added to HttpRequestModel but there are no tests covering its serialization, deserialization, or behavior. Given that other fields in this model have test coverage (as seen in http_request_model_test.dart), this field should have similar coverage to ensure:
- It correctly serializes to/from JSON
- It defaults to the correct value when missing from JSON
- The copyWith method works correctly with it
| enum AuthInheritanceType { | ||
| none("None"), | ||
| environment("Environment"); | ||
|
|
||
| const AuthInheritanceType(this.displayType); | ||
| final String displayType; | ||
| } |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AuthInheritanceType enum lacks documentation explaining what each value means and when to use them. Consider adding documentation:
/// Defines how authentication credentials are determined for a request.
enum AuthInheritanceType {
/// Use request-specific authentication configuration.
none("None"),
/// Inherit authentication from the active environment's default auth.
environment("Environment");
const AuthInheritanceType(this.displayType);
final String displayType;
}| SizedBox(width: 30), | ||
| Text("Variable"), | ||
| SizedBox(width: 30), | ||
| Text("Value"), | ||
| SizedBox(width: 40), |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] These SizedBox widgets should use const for better performance since they have constant dimensions:
const SizedBox(width: 30),
Text("Variable"),
const SizedBox(width: 30),
Text("Value"),
const SizedBox(width: 40),Note: The Text widgets cannot be const as they are not literal strings in this context.
| SizedBox( | ||
| height: 8, | ||
| ), | ||
| ADPopupMenu<APIAuthType>( | ||
| value: authModel?.type.displayType, | ||
| values: APIAuthType.values | ||
| .map((type) => (type, type.displayType)) | ||
| .toList(), | ||
| tooltip: kTooltipSelectAuth, | ||
| isOutlined: true, | ||
| onChanged: readOnly ? null : onChangedAuthType, | ||
| ), | ||
| const SizedBox(height: 48), | ||
| switch (authModel?.type) { | ||
| APIAuthType.basic => BasicAuthFields( | ||
| readOnly: readOnly, | ||
| authData: authModel, | ||
| updateAuth: updateAuthData, | ||
| ), | ||
| APIAuthType.bearer => BearerAuthFields( | ||
| readOnly: readOnly, | ||
| authData: authModel, | ||
| updateAuth: updateAuthData, | ||
| // Auth inheritance selection | ||
| if (!readOnly) ...[ | ||
| ADPopupMenu<AuthInheritanceType>( | ||
| value: (authInheritanceType ?? AuthInheritanceType.none).displayType, | ||
| values: AuthInheritanceType.values | ||
| .map((type) => (type, type.displayType)) | ||
| .toList(), | ||
| tooltip: "Select auth inheritance type", | ||
| isOutlined: true, | ||
| onChanged: onChangedAuthInheritanceType, | ||
| ), | ||
| const SizedBox(height: 16), |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] These SizedBox widgets should use const for better performance:
const SizedBox(height: 8),and
const SizedBox(height: 16),
π Pull Request: Auth Inheritance Feature
π§© PR Description
This PR implements the Auth Inheritance Feature for the APIDash project β enabling requests to inherit authentication credentials from their active environment.
π Overview
This brings Postman/Insomnia-like behavior, where users can define authentication at the environment level and reuse it across multiple requests automatically.
π§ Problem
Previously, APIDash only supported per-request authentication, forcing users to manually configure identical credentials for multiple requests β causing:
π‘ Solution
Auth inheritance adds the ability for requests to use the environmentβs default authentication when selected.
Implemented by:
EnvironmentModelto include default authenticationauthInheritanceTypeinHttpRequestModel(Enum:none,environment)CollectionStateNotifierto apply inherited auth dynamicallyβοΈ Key Technical Changes
defaultAuthModelfield inEnvironmentModelauthInheritanceTypefield (AuthInheritanceTypeenum) inHttpRequestModelAuthInheritanceTypeenum with values:noneβ Use request-specific authenvironmentβ Inherit from environmentCollectionStateNotifierπ§ Benefits
β Reduced Duplication: Single configuration for multiple requests
β Centralized Management: Modify environment-level auth once, apply everywhere
β Familiar Workflow: Mirrors Postman/Insomnia UX
β Backward Compatible: Default behavior unchanged (
none)β Environment Variable Support: Works with inherited credentials
π Related Issues
Ability to inherit auth credentials #881
β Checklist
mainbefore making this PRflutter upgrade)flutter test) and verified they passπ§ͺ Added / Updated Tests
We encourage you to add relevant test cases.
π» OS Used for Development and Testing
π§© References
π§βπ» Author: @orbaps
π Date:
1st November